From 28eaae5955b80ec88bf14b54ab26bbe840aa980e Mon Sep 17 00:00:00 2001 From: John Hobbs Date: Thu, 23 Jan 2025 09:51:12 -0600 Subject: [PATCH 1/3] Account for accessibility change counts in UI --- node-src/types.ts | 1 + .../errors/buildHasChanges.stories.ts | 13 ++++++++++++ .../ui/messages/errors/buildHasChanges.ts | 15 ++++++++++---- node-src/ui/messages/info/buildPassed.ts | 20 ++++++++++++++----- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/node-src/types.ts b/node-src/types.ts index 53df87889..45b664319 100644 --- a/node-src/types.ts +++ b/node-src/types.ts @@ -287,6 +287,7 @@ export interface Context { testCount: number; changeCount: number; errorCount: number; + accessibilityChangeCount: number; interactionTestFailuresCount: number; inProgressCount?: number; autoAcceptChanges: boolean; diff --git a/node-src/ui/messages/errors/buildHasChanges.stories.ts b/node-src/ui/messages/errors/buildHasChanges.stories.ts index ce3d8f9fd..9a260a776 100644 --- a/node-src/ui/messages/errors/buildHasChanges.stories.ts +++ b/node-src/ui/messages/errors/buildHasChanges.stories.ts @@ -8,6 +8,7 @@ const context = { build: { number: 42, changeCount: 2, + accessibilityChangeCount: 1, webUrl: 'https://www.chromatic.com/build?appId=59c59bd0183bd100364e1d57&number=42', app: { setupUrl: 'https://www.chromatic.com/setup?appId=59c59bd0183bd100364e1d57', @@ -19,5 +20,17 @@ const context = { export const BuildHasChangesNotOnboarding = () => buildHasChanges(context); +export const BuildHasChangesVisualOnly = () => + buildHasChanges({ + ...context, + build: { ...context.build, accessibilityChangeCount: 0 }, + }); + +export const BuildHasChangesAccessibilityOnly = () => + buildHasChanges({ + ...context, + build: { ...context.build, changeCount: 0 }, + }); + export const BuildHasChangesIsOnboarding = () => buildHasChanges({ ...context, isOnboarding: true }); diff --git a/node-src/ui/messages/errors/buildHasChanges.ts b/node-src/ui/messages/errors/buildHasChanges.ts index e3959bebb..cbdfce684 100644 --- a/node-src/ui/messages/errors/buildHasChanges.ts +++ b/node-src/ui/messages/errors/buildHasChanges.ts @@ -6,11 +6,18 @@ import { error, info } from '../../components/icons'; import link from '../../components/link'; export default ({ build, exitCode, isOnboarding }) => { - const changes = pluralize('visual changes', build.changeCount, true); + const url = isOnboarding ? build.app.setupUrl : build.webUrl; + + const changes: any[] = []; + if (build.changeCount > 0) { + changes.push(pluralize('visual changes', build.changeCount, true)); + } + if (build.accessibilityChangeCount > 0) { + changes.push(pluralize('accessibility changes', build.accessibilityChangeCount, true)); + } + return dedent(chalk` - ${error} {bold Found ${changes}}: Review the changes at ${link( - isOnboarding ? build.app.setupUrl : build.webUrl - )} + ${error} {bold Found ${changes.join(' and ')}}: Review the changes at ${link(url)} ${info} For CI/CD use cases, this command failed with exit code ${exitCode} Pass {bold --exit-zero-on-changes} to succeed this command regardless of changes. diff --git a/node-src/ui/messages/info/buildPassed.ts b/node-src/ui/messages/info/buildPassed.ts index f0ce7df5f..c1a5f656c 100644 --- a/node-src/ui/messages/info/buildPassed.ts +++ b/node-src/ui/messages/info/buildPassed.ts @@ -9,8 +9,9 @@ import link from '../../components/link'; import { stats } from '../../tasks/snapshot'; export default (ctx: Context) => { - const { changes, snapshots, components, stories, e2eTests } = stats({ build: ctx.build }); - const visualChanges = pluralize('visual changes', ctx.build.changeCount, true); + const { snapshots, components, stories, e2eTests } = stats({ build: ctx.build }); + + const totalChanges = ctx.build.changeCount + ctx.build.accessibilityChangeCount; if (ctx.isOnboarding) { const foundString = isE2EBuild(ctx.options) @@ -23,15 +24,24 @@ export default (ctx: Context) => { ${info} Please continue setup at ${link(ctx.build.app.setupUrl)} `); } - return ctx.build.autoAcceptChanges && ctx.build.changeCount + + const changes: any[] = []; + if (ctx.build.changeCount > 0) { + changes.push(pluralize('visual changes', ctx.build.changeCount, true)); + } + if (ctx.build.accessibilityChangeCount > 0) { + changes.push(pluralize('accessibility changes', ctx.build.accessibilityChangeCount, true)); + } + + return ctx.build.autoAcceptChanges && totalChanges > 0 ? dedent(chalk` ${success} {bold Build ${ctx.build.number} passed!} - Auto-accepted ${changes}. + Auto-accepted ${pluralize('changes', ctx.build.changeCount + ctx.build.accessibilityChangeCount, true)}. ${info} View build details at ${link(ctx.build.webUrl)} `) : dedent(chalk` ${success} {bold Build ${ctx.build.number} passed!} - ${ctx.build.changeCount > 0 ? visualChanges : 'No visual changes'} were found in this build. + ${totalChanges > 0 ? changes.join(' and ') : 'No changes'} were found in this build. ${info} View build details at ${link(ctx.build.webUrl)} `); }; From 33140257ec93a3024de734ed2e6cb1c602845c07 Mon Sep 17 00:00:00 2001 From: John Hobbs Date: Fri, 24 Jan 2025 13:43:32 -0600 Subject: [PATCH 2/3] Move to multiline format for changes. --- .../ui/messages/errors/buildHasChanges.ts | 12 ++++++++--- .../ui/messages/info/buildPassed.stories.ts | 21 +++++++++++++++++++ node-src/ui/messages/info/buildPassed.ts | 4 ++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/node-src/ui/messages/errors/buildHasChanges.ts b/node-src/ui/messages/errors/buildHasChanges.ts index cbdfce684..17b98042e 100644 --- a/node-src/ui/messages/errors/buildHasChanges.ts +++ b/node-src/ui/messages/errors/buildHasChanges.ts @@ -10,14 +10,20 @@ export default ({ build, exitCode, isOnboarding }) => { const changes: any[] = []; if (build.changeCount > 0) { - changes.push(pluralize('visual changes', build.changeCount, true)); + changes.push( + chalk`${error} {bold Found ${pluralize('visual changes', build.changeCount, true)}}` + ); } if (build.accessibilityChangeCount > 0) { - changes.push(pluralize('accessibility changes', build.accessibilityChangeCount, true)); + changes.push( + chalk`${error} {bold Found ${pluralize('accessibility changes', build.accessibilityChangeCount, true)}}` + ); } return dedent(chalk` - ${error} {bold Found ${changes.join(' and ')}}: Review the changes at ${link(url)} + ${changes.join('\n')} + + Review the changes at ${link(url)} ${info} For CI/CD use cases, this command failed with exit code ${exitCode} Pass {bold --exit-zero-on-changes} to succeed this command regardless of changes. diff --git a/node-src/ui/messages/info/buildPassed.stories.ts b/node-src/ui/messages/info/buildPassed.stories.ts index 2140561cc..4ca33d2e1 100644 --- a/node-src/ui/messages/info/buildPassed.stories.ts +++ b/node-src/ui/messages/info/buildPassed.stories.ts @@ -23,6 +23,27 @@ export const BuildPassedWithChanges = () => number: 42, webUrl: 'https://www.chromatic.com/build?appId=59c59bd0183bd100364e1d57&number=42', changeCount: 2, + accessibilityChangeCount: 1, + }, + } as any); + +export const BuildPassedWithVisualChanges = () => + buildPassed({ + ...ctx, + build: { + number: 42, + webUrl: 'https://www.chromatic.com/build?appId=59c59bd0183bd100364e1d57&number=42', + changeCount: 2, + }, + } as any); + +export const BuildPassedWithAccessibilityChanges = () => + buildPassed({ + ...ctx, + build: { + number: 42, + webUrl: 'https://www.chromatic.com/build?appId=59c59bd0183bd100364e1d57&number=42', + accessibilityChangeCount: 1, }, } as any); diff --git a/node-src/ui/messages/info/buildPassed.ts b/node-src/ui/messages/info/buildPassed.ts index c1a5f656c..a38a86c20 100644 --- a/node-src/ui/messages/info/buildPassed.ts +++ b/node-src/ui/messages/info/buildPassed.ts @@ -11,7 +11,7 @@ import { stats } from '../../tasks/snapshot'; export default (ctx: Context) => { const { snapshots, components, stories, e2eTests } = stats({ build: ctx.build }); - const totalChanges = ctx.build.changeCount + ctx.build.accessibilityChangeCount; + const totalChanges = (ctx.build.changeCount || 0) + (ctx.build.accessibilityChangeCount || 0); if (ctx.isOnboarding) { const foundString = isE2EBuild(ctx.options) @@ -36,7 +36,7 @@ export default (ctx: Context) => { return ctx.build.autoAcceptChanges && totalChanges > 0 ? dedent(chalk` ${success} {bold Build ${ctx.build.number} passed!} - Auto-accepted ${pluralize('changes', ctx.build.changeCount + ctx.build.accessibilityChangeCount, true)}. + Auto-accepted ${pluralize('changes', totalChanges, true)}. ${info} View build details at ${link(ctx.build.webUrl)} `) : dedent(chalk` From 7af2d57b81cbe720564aa833ec54f80d2ee9d665 Mon Sep 17 00:00:00 2001 From: John Hobbs Date: Fri, 24 Jan 2025 16:03:37 -0600 Subject: [PATCH 3/3] Pluralize was/were on total changes. --- node-src/ui/messages/info/buildPassed.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node-src/ui/messages/info/buildPassed.ts b/node-src/ui/messages/info/buildPassed.ts index a38a86c20..9756e456e 100644 --- a/node-src/ui/messages/info/buildPassed.ts +++ b/node-src/ui/messages/info/buildPassed.ts @@ -41,7 +41,7 @@ export default (ctx: Context) => { `) : dedent(chalk` ${success} {bold Build ${ctx.build.number} passed!} - ${totalChanges > 0 ? changes.join(' and ') : 'No changes'} were found in this build. + ${totalChanges > 0 ? changes.join(' and ') : 'No changes'} ${pluralize('was', totalChanges, false)} found in this build. ${info} View build details at ${link(ctx.build.webUrl)} `); };