-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Merge S1 and S2 chromatic storybooks #6965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7525e66
838482b
64731a0
391f2d4
1df6f3d
72b86e1
2105607
1efa254
6c9fb4b
28ffdae
058e425
1cee66f
99b388e
06be29b
46fc929
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,10 @@ | ||
| import {expressThemes, locales, scales, themes} from '../../constants'; | ||
| import {expressThemes, locales, S2Backgrounds, S2ColorThemes, scales, themes} from '../../constants'; | ||
| import {makeDecorator} from '@storybook/preview-api'; | ||
| import {Provider, View} from '@adobe/react-spectrum'; | ||
| import {Provider as S2Provider} from '@react-spectrum/s2'; | ||
| import React, {useEffect} from 'react'; | ||
| // TODO: can't seem to use the style macro here... | ||
| // import {style} from '../../../packages/@react-spectrum/s2/style/spectrum-theme' with {type: 'macro'}; | ||
| import './disableAnimations.css'; | ||
|
|
||
| export const withChromaticProvider = makeDecorator({ | ||
|
|
@@ -15,8 +18,7 @@ export const withChromaticProvider = makeDecorator({ | |
| } else { | ||
| selectedLocales = options.locales ? locales.map(l => l.value).slice(1) : ['en-US', 'ar-AE']; | ||
| } | ||
| let colorSchemes = options.express ? [] : (options.colorSchemes || Object.keys(themes)); | ||
| let scalesToRender = options.scales || Object.keys(scales); | ||
|
|
||
| let height; | ||
| let minHeight; | ||
| if(isNaN(options.height)) { | ||
|
|
@@ -25,41 +27,77 @@ export const withChromaticProvider = makeDecorator({ | |
| height = options.height; | ||
| } | ||
|
|
||
| let expressTheme = colorSchemes.length === 1 ? expressThemes[colorSchemes[0]] : expressThemes.light; | ||
| let expressColorScheme = colorSchemes.length === 1 ? colorSchemes[0].replace(/est$/, '') : 'light'; | ||
| let expressScale = scalesToRender.length === 1 ? scalesToRender[0] : 'medium'; | ||
| let expressLocale = selectedLocales.length === 1 ? selectedLocales[0] : 'en-US'; | ||
|
|
||
| // do not add a top level provider, each provider variant needs to be independent so that we don't have RTL/LTR styles that interfere with each other | ||
| return ( | ||
| <DisableAnimations disableAnimations={options.disableAnimations}> | ||
| <div style={{display: 'flex', flexDirection: 'column', height, minHeight}}> | ||
| {colorSchemes.map(colorScheme => | ||
| scalesToRender.map(scale => | ||
| (colorScheme === 'light' ? selectedLocales : ['en-US']).map(locale => | ||
| <Provider key={`${colorScheme}_${scale}_${locale}`} theme={themes[colorScheme]} colorScheme={colorScheme.replace(/est$/, '')} scale={scale} locale={locale} typekitId="pbi5ojv"> | ||
| <View margin="size-100"> | ||
| <h1 style={{margin: 0, padding: 0}}>{`${colorScheme}, ${scale}, ${locale}`}</h1> | ||
| {getStory(context)} | ||
| </View> | ||
| </Provider> | ||
| ) | ||
| ) | ||
| )} | ||
| {options.express !== false && | ||
| <Provider key="express" theme={expressTheme} colorScheme={expressColorScheme} scale={expressScale} locale={expressLocale} typekitId="pbi5ojv"> | ||
| <View margin="size-100"> | ||
| <h1 style={{margin: 0, padding: 0}}>express, {expressColorScheme}, {expressScale}, {expressLocale}</h1> | ||
| {getStory(context)} | ||
| </View> | ||
| </Provider> | ||
| } | ||
| </div> | ||
| </DisableAnimations> | ||
| ) | ||
| if (context.title.includes('S2')) { | ||
| return <RenderS2 getStory={getStory} context={context} options={options} selectedLocales={selectedLocales} height={height} minHeight={minHeight} /> | ||
| } else { | ||
| return <RenderV3 getStory={getStory} context={context} options={options} selectedLocales={selectedLocales} height={height} minHeight={minHeight} /> | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| function RenderS2({getStory, context, options, selectedLocales, height, minHeight}) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you make this into a shared component that .chromatic-fc and .chromatic both use? I assume there is no way you could make this general enough to make it not specific to s2 or RSPv3, the s2 or RSP3 parts are passed in, or toggles between them based on a property? It's just a big ugly block of code getting repeated. It's probably not worth the effort to refactor it this way. As part of this, I was wondering if making it a component would move the style macro import out of this file and into another file where it might work.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for now I'll leave it as its own block until I figure out the full set of differences when I add the powerset prop combination stories for s2, but yeah it would be nice to unify this |
||
| let colorSchemes = options.colorSchemes || S2ColorThemes; | ||
| let backgrounds = options.backgrounds || ['base']; | ||
|
|
||
| // TODO: there is perhaps some things that can still be shared between the two renders but figured it be easiest to just split them out for the most part | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can remove todo |
||
| return ( | ||
| <DisableAnimations disableAnimations={options.disableAnimations}> | ||
| <div style={{display: 'flex', flexDirection: 'column', height, minHeight, width: '90vw'}}> | ||
| {colorSchemes.map(colorScheme => | ||
| backgrounds.map(background => | ||
| (colorScheme === 'light' || context.title.includes('RTL') ? selectedLocales : ['en-US']).map(locale => | ||
| <S2Provider key={`${colorScheme}_${background}_${locale}`} background={background} colorScheme={colorScheme} locale={locale}> | ||
| <div style={{margin: '8px'}}> | ||
| <h1 style={{margin: 0, padding: 0, color: colorScheme === 'dark' ? 'white' : 'black'}}>{`${colorScheme}, ${background}, ${locale}`}</h1> | ||
| {getStory(context)} | ||
| </div> | ||
| </S2Provider> | ||
| ) | ||
| ) | ||
| )} | ||
| </div> | ||
| </DisableAnimations> | ||
| ) | ||
| } | ||
|
|
||
| function RenderV3({getStory, context, options, selectedLocales, height, minHeight}) { | ||
| let colorSchemes = options.express ? [] : (options.colorSchemes || Object.keys(themes)); | ||
| let scalesToRender = options.scales || Object.keys(scales); | ||
|
|
||
| let expressTheme = colorSchemes.length === 1 ? expressThemes[colorSchemes[0]] : expressThemes.light; | ||
| let expressColorScheme = colorSchemes.length === 1 ? colorSchemes[0].replace(/est$/, '') : 'light'; | ||
| let expressScale = scalesToRender.length === 1 ? scalesToRender[0] : 'medium'; | ||
| let expressLocale = selectedLocales.length === 1 ? selectedLocales[0] : 'en-US'; | ||
|
|
||
| // do not add a top level provider, each provider variant needs to be independent so that we don't have RTL/LTR styles that interfere with each other | ||
| return ( | ||
| <DisableAnimations disableAnimations={options.disableAnimations}> | ||
| <div style={{display: 'flex', flexDirection: 'column', height, minHeight}}> | ||
| {colorSchemes.map(colorScheme => | ||
| scalesToRender.map(scale => | ||
| (colorScheme === 'light' ? selectedLocales : ['en-US']).map(locale => | ||
| <Provider key={`${colorScheme}_${scale}_${locale}`} theme={themes[colorScheme]} colorScheme={colorScheme.replace(/est$/, '')} scale={scale} locale={locale} typekitId="pbi5ojv"> | ||
| <View margin="size-100"> | ||
| <h1 style={{margin: 0, padding: 0}}>{`${colorScheme}, ${scale}, ${locale}`}</h1> | ||
| {getStory(context)} | ||
| </View> | ||
| </Provider> | ||
| ) | ||
| ) | ||
| )} | ||
| {options.express !== false && | ||
| <Provider key="express" theme={expressTheme} colorScheme={expressColorScheme} scale={expressScale} locale={expressLocale} typekitId="pbi5ojv"> | ||
| <View margin="size-100"> | ||
| <h1 style={{margin: 0, padding: 0}}>express, {expressColorScheme}, {expressScale}, {expressLocale}</h1> | ||
| {getStory(context)} | ||
| </View> | ||
| </Provider> | ||
| } | ||
| </div> | ||
| </DisableAnimations> | ||
| ) | ||
| } | ||
|
|
||
| function DisableAnimations({children, disableAnimations}) { | ||
| useEffect(() => { | ||
| if (disableAnimations) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly sure why the macro doesn't work here, ideally I'd use if for some of the divs below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't import directly from
@react-spectrum/s2either? instead of the relative path?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, doing that returns an error of
The relative path way returns an error of
I haven't had time to dig into too deeply but its not a huge deal IMO since it is just some padding/div wrappers for story layout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
o, maybe this is loaded through another bundler, storybook UI has one vs the one used for stories