From 0cf1f0ca7f64e44728ab122e3f7562da7b6b5042 Mon Sep 17 00:00:00 2001 From: Valery Bugakov Date: Wed, 28 Apr 2021 22:19:55 +0800 Subject: [PATCH] web: enable cross-theme screenshots in Chromatic (#20349) * web: enable cross-theme screenshots in Chromatic * web: use useRedesignToggle in create-chromatic-story * web: remove redundant condition * web: add additional comments * web: remove unused var * web: add comment * web: add @storybook/client-api to dev deps * web: handle initial redesign-theme class toggle in toolbar component * web: remove redundant import * Update client/branded/src/global-styles/colors-redesign.scss Co-authored-by: Felix Becker * web: use for of loop * web: update renovate.md * web: disable Chromatic workflow if no related files were changed * web: disable renovate rebase on conflict to limit the number of Chromatic uploads * web: update comment * Update enterprise/dev/ci/internal/ci/is-storybook-affected.go Co-authored-by: Tom Ross * web: do not ignore .graphqlrc.yml and run Chromatic if isMasterDryRun Co-authored-by: Felix Becker Co-authored-by: Tom Ross --- .../src/global-styles/colors-redesign.scss | 4 +- .../shared/src/util/addRedesignVariants.tsx | 27 -------- .../src/chromatic-story/Chromatic.story.tsx | 37 +++++++++++ .../src/chromatic-story/add-story.ts | 60 ++++++++++++++++++ .../create-chromatic-story.tsx | 47 ++++++++++++++ client/storybook/src/main.ts | 6 +- client/storybook/src/preview.ts | 10 +-- .../RedesignToggleStorybook.tsx | 26 ++++++-- client/storybook/src/themes.ts | 7 ++- doc/dev/background-information/renovate.md | 4 +- .../ci/internal/ci/is-storybook-affected.go | 63 +++++++++++++++++++ .../dev/ci/internal/ci/pipeline-steps.go | 22 ++++--- package.json | 1 + renovate.json | 1 + yarn.lock | 2 +- 15 files changed, 262 insertions(+), 55 deletions(-) delete mode 100644 client/shared/src/util/addRedesignVariants.tsx create mode 100644 client/storybook/src/chromatic-story/Chromatic.story.tsx create mode 100644 client/storybook/src/chromatic-story/add-story.ts create mode 100644 client/storybook/src/chromatic-story/create-chromatic-story.tsx create mode 100644 enterprise/dev/ci/internal/ci/is-storybook-affected.go diff --git a/client/branded/src/global-styles/colors-redesign.scss b/client/branded/src/global-styles/colors-redesign.scss index f3b7ec4235d..c6ae64b0b56 100644 --- a/client/branded/src/global-styles/colors-redesign.scss +++ b/client/branded/src/global-styles/colors-redesign.scss @@ -35,9 +35,7 @@ $theme-colors-redesign: ( merged: $redesign-purple, ); -:root.theme-redesign, -// Descendant selector is required for Storybook `addRedesignVariants` helper. -:root .theme-redesign { +.theme-redesign { --primary: #{map-get($theme-colors-redesign, 'primary')}; --primary-3: #{$redesign-indigo}; --brand-secondary: #a305e1; diff --git a/client/shared/src/util/addRedesignVariants.tsx b/client/shared/src/util/addRedesignVariants.tsx deleted file mode 100644 index 265b3b02f01..00000000000 --- a/client/shared/src/util/addRedesignVariants.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import React, { ReactElement } from 'react' - -import { REDESIGN_CLASS_NAME } from './useRedesignToggle' - -/** - * - * To rely on screenshot tests for redesigned components in Storybook, this wrapper function - * renders a copy of the story wrapped into a div with redesign class next to the initial story. - * - * @example - * const { add } = storiesOf('wildcard/PageSelector', module).addDecorator(story => ( - * - * {() => addRedesignVariants(
{story()}
)} - * - * )) - * - */ -export const addRedesignVariants = (story: ReactElement): ReactElement => ( - <> - {story} -
-
-
Redesign variant
- {story} -
- -) diff --git a/client/storybook/src/chromatic-story/Chromatic.story.tsx b/client/storybook/src/chromatic-story/Chromatic.story.tsx new file mode 100644 index 00000000000..144dae651fc --- /dev/null +++ b/client/storybook/src/chromatic-story/Chromatic.story.tsx @@ -0,0 +1,37 @@ +import { PublishedStoreItem } from '@storybook/client-api' +import { raw } from '@storybook/react' +import isChromatic from 'chromatic/isChromatic' + +import { addStory } from './add-story' + +// Execute logic below only in the environment where Chromatic snapshots are captured. +if (isChromatic()) { + // Get an array of all stories which are already added to the `StoryStore`. + // Use `raw()` because we don't want to apply any filtering and sorting on the array of stories. + const storeItems = raw() as PublishedStoreItem[] + + // Add three more versions of each story to test visual regressions with Chromatic snapshots. + // In other environments, these themes can be explored by a user via toolbar toggles. + for (const storeItem of storeItems) { + // Default theme + Dark mode. + addStory({ + storeItem, + isDarkModeEnabled: true, + isRedesignEnabled: false, + }) + + // Redesign theme + Light mode. + addStory({ + storeItem, + isDarkModeEnabled: false, + isRedesignEnabled: true, + }) + + // Redesign theme + Dark mode. + addStory({ + storeItem, + isDarkModeEnabled: true, + isRedesignEnabled: true, + }) + } +} diff --git a/client/storybook/src/chromatic-story/add-story.ts b/client/storybook/src/chromatic-story/add-story.ts new file mode 100644 index 00000000000..3ce2cc095aa --- /dev/null +++ b/client/storybook/src/chromatic-story/add-story.ts @@ -0,0 +1,60 @@ +import { PublishedStoreItem, StoryStore } from '@storybook/client-api' +import { toId } from '@storybook/csf' + +import { createChromaticStory, CreateChromaticStoryOptions } from './create-chromatic-story' + +// This global reference is used internally by Storybook: +// https://github.com/storybookjs/storybook/blob/3ec358f71c6111838092397d13fbe35b627a9a9d/lib/core-client/src/preview/start.ts#L43 +declare global { + interface Window { + __STORYBOOK_STORY_STORE__: StoryStore + } +} + +// See the discussion about `StoryStore` usage in stories: +// https://github.com/storybookjs/storybook/discussions/12050#discussioncomment-125658 +const storyStore = window.__STORYBOOK_STORY_STORE__ + +interface AddStoryOptions extends Pick { + storeItem: PublishedStoreItem +} + +export const addStory = (options: AddStoryOptions): void => { + const { + storeItem: { name, kind, storyFn, parameters }, + isDarkModeEnabled, + isRedesignEnabled, + } = options + + // Add suffix to the story name based on theme options: + // 1. Default + Dark: "Text" -> "Text 🌚" + // 2. Redesign + Light: "Text" -> "Text [Redesign]" + // 3. Redesign + Dark: "Text" -> "Text [Redesign] 🌚" + const storyName = [name, isRedesignEnabled && '[Redesign]', isDarkModeEnabled && '🌚'].filter(Boolean).join(' ') + + /** + * Use `storyStore.addStory()` to avoid applying decorators to stories, because `PublishedStoreItem.storyFn` already has decorators applied. + * `storiesOf().add()` usage API would result in decorators duplication. It's possible to avoid this issue using `PublishedStoreItem.getOriginal()`, + * which returns only story function without any decorators and story context. It means that we should apply them manually and + * keep this logic in sync with Storybook internals to have consistent behavior. `storyStore.addStory()` allows to avoid it. + */ + storyStore.addStory( + { + id: toId(kind, storyName), + kind, + name: storyName, + parameters, + loaders: [], + storyFn: createChromaticStory({ + storyFn, + isDarkModeEnabled, + isRedesignEnabled, + }), + }, + { + // The default `applyDecorators` implementation accepts `decorators` as a second arg and applies them to the `storyFn`. + // Our `storyFn` already has all the decorators applied, so we just return it. + applyDecorators: storyFn => storyFn, + } + ) +} diff --git a/client/storybook/src/chromatic-story/create-chromatic-story.tsx b/client/storybook/src/chromatic-story/create-chromatic-story.tsx new file mode 100644 index 00000000000..17d17a605ce --- /dev/null +++ b/client/storybook/src/chromatic-story/create-chromatic-story.tsx @@ -0,0 +1,47 @@ +import { StoryFn } from '@storybook/addons' +import React, { ReactElement, useEffect } from 'react' +import { useDarkMode } from 'storybook-dark-mode' + +import { useRedesignToggle } from '@sourcegraph/shared/src/util/useRedesignToggle' + +import { THEME_DARK_CLASS, THEME_LIGHT_CLASS } from '../themes' + +export interface CreateChromaticStoryOptions { + storyFn: StoryFn + isRedesignEnabled: boolean + isDarkModeEnabled: boolean +} + +// Wrap `storyFn` into a decorator which takes care of CSS classes toggling based on received theme options. +export const createChromaticStory = (options: CreateChromaticStoryOptions): StoryFn => () => { + const { storyFn, isRedesignEnabled, isDarkModeEnabled } = options + // The `storyFn` is retrieved from the `StoryStore`, so it already has a `StoryContext`. + // We can safely change its type to remove required props `StoryContext` props check. + const Story = storyFn as React.ComponentType + + const [, setRedesignToggle] = useRedesignToggle() + const isDarkModeEnabledInitially = useDarkMode() + + useEffect(() => { + setRedesignToggle(isRedesignEnabled) + + // 'storybook-dark-mode' doesn't expose any API to toggle dark/light theme programmatically, so we do it manually. + document.body.classList.toggle(THEME_DARK_CLASS, isDarkModeEnabled) + document.body.classList.toggle(THEME_LIGHT_CLASS, !isDarkModeEnabled) + + return () => { + // Do not enable redesign theme if it was disabled before this story was opened. + if (isRedesignEnabled) { + setRedesignToggle(!isRedesignEnabled) + } + + // Always toggle dark mode back to the previous value because otherwise, it might be out of sync with the toolbar toggle. + document.body.classList.toggle(THEME_DARK_CLASS, isDarkModeEnabledInitially) + document.body.classList.toggle(THEME_LIGHT_CLASS, !isDarkModeEnabledInitially) + } + // We need to execute `useEffect` callback once to take snapshot in Chromatic, so we can omit dependencies here. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) + + return +} diff --git a/client/storybook/src/main.ts b/client/storybook/src/main.ts index 63b277aa9f9..6f7e7ea430e 100644 --- a/client/storybook/src/main.ts +++ b/client/storybook/src/main.ts @@ -7,6 +7,10 @@ import { Configuration, DefinePlugin, ProgressPlugin, RuleSetUseItem, RuleSetUse const rootPath = path.resolve(__dirname, '../../../') const monacoEditorPaths = [path.resolve(rootPath, 'node_modules', 'monaco-editor')] + +// Stories in this file are guarded by the `isChromatic()` check. It will result in noop in all other environments. +const chromaticStoriesGlob = path.resolve(rootPath, 'client/storybook/src/chromatic-story/Chromatic.story.tsx') + // Due to an issue with constant recompiling (https://github.com/storybookjs/storybook/issues/14342) // we need to make the globs more specific (`(web|shared..)` also doesn't work). Once the above issue // is fixed, this can be removed and watched for `client/**/*.story.tsx` again. @@ -32,7 +36,7 @@ const getCSSLoaders = (...loaders: RuleSetUseItem[]): RuleSetUse => [ ] const config = { - stories: storiesGlobs, + stories: [...storiesGlobs, chromaticStoriesGlob], addons: [ '@storybook/addon-knobs', '@storybook/addon-actions', diff --git a/client/storybook/src/preview.ts b/client/storybook/src/preview.ts index afcf05797db..ca233eabe8d 100644 --- a/client/storybook/src/preview.ts +++ b/client/storybook/src/preview.ts @@ -9,7 +9,7 @@ import { withDesign } from 'storybook-addon-designs' import { setLinkComponent, AnchorLink } from '@sourcegraph/shared/src/components/Link' import { getIsRedesignEnabled, REDESIGN_CLASS_NAME } from '@sourcegraph/shared/src/util/useRedesignToggle' -import * as themes from './themes' +import { themeDark, themeLight, THEME_DARK_CLASS, THEME_LIGHT_CLASS } from './themes' const withConsoleDecorator: DecoratorFunction = (storyFn, context): ReactElement => withConsole()(storyFn)(context) @@ -19,10 +19,10 @@ export const decorators = [withDesign, withConsoleDecorator] export const parameters = { darkMode: { stylePreview: true, - darkClass: 'theme-dark', - lightClass: 'theme-light', - light: themes.light, - dark: themes.dark, + lightClass: THEME_LIGHT_CLASS, + darkClass: THEME_DARK_CLASS, + light: themeLight, + dark: themeDark, }, } diff --git a/client/storybook/src/redesign-toggle-toolbar/RedesignToggleStorybook.tsx b/client/storybook/src/redesign-toggle-toolbar/RedesignToggleStorybook.tsx index 970004c248e..900c014c0e0 100644 --- a/client/storybook/src/redesign-toggle-toolbar/RedesignToggleStorybook.tsx +++ b/client/storybook/src/redesign-toggle-toolbar/RedesignToggleStorybook.tsx @@ -1,10 +1,12 @@ +import { addons } from '@storybook/addons' import { Icons, IconButton } from '@storybook/components' -import React, { ReactElement } from 'react' +import { SET_STORIES } from '@storybook/core-events' +import React, { ReactElement, useEffect } from 'react' import { useRedesignToggle, REDESIGN_CLASS_NAME } from '@sourcegraph/shared/src/util/useRedesignToggle' const toggleRedesignClass = (element: HTMLElement, isRedesignEnabled: boolean): void => { - element.classList.toggle(REDESIGN_CLASS_NAME, !isRedesignEnabled) + element.classList.toggle(REDESIGN_CLASS_NAME, isRedesignEnabled) } const updatePreview = (isRedesignEnabled: boolean): void => { @@ -29,10 +31,26 @@ const updateManager = (isRedesignEnabled: boolean): void => { export const RedesignToggleStorybook = (): ReactElement => { const [isRedesignEnabled, setIsRedesignEnabled] = useRedesignToggle() + useEffect(() => { + const handleIsRedesignEnabledChange = (): void => { + updatePreview(isRedesignEnabled) + updateManager(isRedesignEnabled) + } + + handleIsRedesignEnabledChange() + + const channel = addons.getChannel() + // Preview iframe is not available on toolbar mount. + // Wait for the SET_STORIES event, after which the iframe is accessible, and ensure that the redesign-theme class is in place. + channel.on(SET_STORIES, handleIsRedesignEnabledChange) + + return () => { + channel.removeListener(SET_STORIES, handleIsRedesignEnabledChange) + } + }, [isRedesignEnabled]) + const handleRedesignToggle = (): void => { setIsRedesignEnabled(!isRedesignEnabled) - updatePreview(isRedesignEnabled) - updateManager(isRedesignEnabled) } return ( diff --git a/client/storybook/src/themes.ts b/client/storybook/src/themes.ts index d76e37be71d..4d2f1c016d1 100644 --- a/client/storybook/src/themes.ts +++ b/client/storybook/src/themes.ts @@ -1,6 +1,9 @@ import { ThemeVars, themes } from '@storybook/theming' import openColor from 'open-color' +export const THEME_DARK_CLASS = 'theme-dark' +export const THEME_LIGHT_CLASS = 'theme-light' + // Themes use the colors from our webapp. const common: Omit = { colorPrimary: openColor.blue[6], @@ -13,7 +16,7 @@ const common: Omit = { fontCode: 'sfmono-regular, consolas, menlo, dejavu sans mono, monospace', } -export const dark: ThemeVars = { +export const themeDark: ThemeVars = { ...themes.dark, ...common, appBg: '#1c2736', @@ -25,7 +28,7 @@ export const dark: ThemeVars = { inputTextColor: '#ffffff', } -export const light: ThemeVars = { +export const themeLight: ThemeVars = { ...themes.light, ...common, appBg: '#fbfdff', diff --git a/doc/dev/background-information/renovate.md b/doc/dev/background-information/renovate.md index c3f9156a987..4f03adac46b 100644 --- a/doc/dev/background-information/renovate.md +++ b/doc/dev/background-information/renovate.md @@ -41,8 +41,8 @@ We heavily customize Renovate to save more time. Possible configurations include - Setting different reviewers for certain dependencies - Grouping certain dependencies -- Automerging certain low-risk dependencies -- Updating certain dependencies out-of-schedule aswell +- Auto merging certain low-risk dependencies +- Updating certain dependencies out-of-schedule as well - Assigning certain labels for easier filtering If you see an opportunity to improve the configuration, raise a pull request to update the `renovate.json` in the repository or our [configuration shared between repositories](https://github.com/sourcegraph/renovate-config/blob/master/renovate.json). diff --git a/enterprise/dev/ci/internal/ci/is-storybook-affected.go b/enterprise/dev/ci/internal/ci/is-storybook-affected.go new file mode 100644 index 00000000000..b8e90333ef3 --- /dev/null +++ b/enterprise/dev/ci/internal/ci/is-storybook-affected.go @@ -0,0 +1,63 @@ +package ci + +import ( + "path/filepath" + "strings" +) + +// Changes in the files below will be ignored by the Storybook workflow. +var ignoredRootFiles []string = []string{ + "jest.config.base.js", + "graphql-schema-linter.config.js", + "libsqlite3-pcre.dylib", + ".mocharc.js", + "go.mod", + "LICENSE", + "renovate.json", + "jest.config.js", + "LICENSE.apache", + ".stylelintrc.json", + ".percy.yml", + ".tool-versions", + "go.sum", + ".golangci.yml", + ".stylelintignore", + ".gitmodules", + ".prettierignore", + ".editorconfig", + "prettier.config.js", + ".dockerignore", + "doc.go", + ".gitignore", + ".gitattributes", + ".eslintrc.js", + "sg.config.yaml", + ".eslintignore", + ".mailmap", + "LICENSE.enterprise", + "CODENOTIFY", +} + +func contains(s []string, str string) bool { + for _, v := range s { + if v == str { + return true + } + } + + return false +} + +func isAllowedRootFile(p string) bool { + return filepath.Dir(p) == "." && !contains(ignoredRootFiles, p) +} + +// Run Storybook workflow only if related files were changed. +func (c Config) isStorybookAffected() bool { + for _, p := range c.changedFiles { + if !strings.HasSuffix(p, ".md") && (strings.HasPrefix(p, "client/") || isAllowedRootFile(p)) { + return true + } + } + return false +} diff --git a/enterprise/dev/ci/internal/ci/pipeline-steps.go b/enterprise/dev/ci/internal/ci/pipeline-steps.go index aab872ded43..70c4ab5b502 100644 --- a/enterprise/dev/ci/internal/ci/pipeline-steps.go +++ b/enterprise/dev/ci/internal/ci/pipeline-steps.go @@ -90,17 +90,19 @@ func addSharedTests(c Config) func(pipeline *bk.Pipeline) { bk.Cmd("dev/ci/codecov.sh -c -F typescript -F integration"), bk.ArtifactPaths("./puppeteer/*.png")) - // Upload storybook to Chromatic - chromaticCommand := "yarn chromatic --exit-zero-on-changes --exit-once-uploaded" - if !c.isPR() { - chromaticCommand += " --auto-accept-changes" + if c.isMasterDryRun || c.isStorybookAffected() { + // Upload storybook to Chromatic + chromaticCommand := "yarn chromatic --exit-zero-on-changes --exit-once-uploaded" + if !c.isPR() { + chromaticCommand += " --auto-accept-changes" + } + pipeline.AddStep(":chromatic: Upload storybook to Chromatic", + bk.AutomaticRetry(5), + bk.Cmd("yarn --mutex network --frozen-lockfile --network-timeout 60000"), + bk.Cmd("yarn gulp generate"), + bk.Env("MINIFY", "1"), + bk.Cmd(chromaticCommand)) } - pipeline.AddStep(":chromatic: Upload storybook to Chromatic", - bk.AutomaticRetry(5), - bk.Cmd("yarn --mutex network --frozen-lockfile --network-timeout 60000"), - bk.Cmd("yarn gulp generate"), - bk.Env("MINIFY", "1"), - bk.Cmd(chromaticCommand)) // Shared tests pipeline.AddStep(":jest: Test shared client code", diff --git a/package.json b/package.json index 34aeb005b74..464d5ac3cec 100644 --- a/package.json +++ b/package.json @@ -121,6 +121,7 @@ "@storybook/addon-toolbars": "^6.2.9", "@storybook/addons": "^6.2.9", "@storybook/components": "^6.2.9", + "@storybook/client-api": "^6.2.9", "@storybook/core": "^6.2.9", "@storybook/react": "^6.2.9", "@storybook/theming": "^6.2.9", diff --git a/renovate.json b/renovate.json index acc7e9901f7..984d40204d9 100644 --- a/renovate.json +++ b/renovate.json @@ -2,6 +2,7 @@ "$schema": "http://json.schemastore.org/renovate", "extends": ["github>sourcegraph/renovate-config"], "semanticCommits": false, + "rebaseWhen": "never", "packageRules": [ { "matchDepTypes": ["engines"], diff --git a/yarn.lock b/yarn.lock index ca71abd95f8..7671beb0f1e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3275,7 +3275,7 @@ ts-dedent "^2.0.0" util-deprecate "^1.0.2" -"@storybook/client-api@6.2.9": +"@storybook/client-api@6.2.9", "@storybook/client-api@^6.2.9": version "6.2.9" resolved "https://registry.npmjs.org/@storybook/client-api/-/client-api-6.2.9.tgz#f0bb44e9b2692adfbf30d7ff751c6dd44bcfe1ce" integrity sha512-aLvEUVkbvv6Qo/2mF4rFCecdqi2CGOUDdsV1a6EFIVS/9gXFdpirsOwKHo9qNjacGdWPlBYGCUcbrw+DvNaSFA==