chore(storybook): Add theme picker to storybook toolbar - #20346
Conversation
Chromatic stores published storybooks with commits, we can simply build an azure function to keep the version -> commit mapping and integrate a picker into the docs page
Adds the theme picker to the storybook toolbar for internal development. The global types are exported since they will be used for the docs page.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 69f0aee:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 050f89bf004c58652909e5dfd3c814383c40e9c5 (build) |
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1046 | 1069 | 5000 | |
| BaseButton | mount | 1059 | 1046 | 5000 | |
| Breadcrumb | mount | 2795 | 2790 | 1000 | |
| ButtonNext | mount | 537 | 555 | 5000 | |
| Checkbox | mount | 1765 | 1743 | 5000 | |
| CheckboxBase | mount | 1477 | 1459 | 5000 | |
| ChoiceGroup | mount | 5244 | 5281 | 5000 | |
| ComboBox | mount | 1050 | 1064 | 1000 | |
| CommandBar | mount | 10752 | 10720 | 1000 | |
| ContextualMenu | mount | 6802 | 6929 | 1000 | |
| DefaultButton | mount | 1232 | 1290 | 5000 | |
| DetailsRow | mount | 4150 | 4177 | 5000 | |
| DetailsRowFast | mount | 4142 | 4108 | 5000 | |
| DetailsRowNoStyles | mount | 3995 | 3958 | 5000 | |
| Dialog | mount | 2727 | 2693 | 1000 | |
| DocumentCardTitle | mount | 185 | 187 | 1000 | |
| Dropdown | mount | 3517 | 3548 | 5000 | |
| FluentProviderNext | mount | 3447 | 3347 | 5000 | |
| FluentProviderWithTheme | mount | 200 | 216 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 98 | 104 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 263 | 235 | 10 | |
| FocusTrapZone | mount | 1960 | 1944 | 5000 | |
| FocusZone | mount | 1905 | 1934 | 5000 | |
| IconButton | mount | 1952 | 2003 | 5000 | |
| Label | mount | 381 | 385 | 5000 | |
| Layer | mount | 3247 | 3235 | 5000 | |
| Link | mount | 543 | 532 | 5000 | |
| MakeStyles | mount | 1911 | 1906 | 50000 | |
| MenuButton | mount | 1680 | 1694 | 5000 | |
| MessageBar | mount | 2141 | 2151 | 5000 | |
| Nav | mount | 3616 | 3572 | 1000 | |
| OverflowSet | mount | 1211 | 1214 | 5000 | |
| Panel | mount | 2557 | 2584 | 1000 | |
| Persona | mount | 912 | 926 | 1000 | |
| Pivot | mount | 1570 | 1585 | 1000 | |
| PrimaryButton | mount | 1426 | 1473 | 5000 | |
| Rating | mount | 8695 | 8651 | 5000 | |
| SearchBox | mount | 1518 | 1521 | 5000 | |
| Shimmer | mount | 2904 | 2893 | 5000 | |
| Slider | mount | 2158 | 2135 | 5000 | |
| SpinButton | mount | 5443 | 5426 | 5000 | |
| Spinner | mount | 465 | 483 | 5000 | |
| SplitButton | mount | 3539 | 3522 | 5000 | |
| Stack | mount | 577 | 581 | 5000 | |
| StackWithIntrinsicChildren | mount | 2004 | 1978 | 5000 | |
| StackWithTextChildren | mount | 5364 | 5331 | 5000 | |
| SwatchColorPicker | mount | 11359 | 11400 | 5000 | |
| TagPicker | mount | 2923 | 2965 | 5000 | |
| TeachingBubble | mount | 13707 | 13670 | 5000 | |
| Text | mount | 483 | 464 | 5000 | |
| TextField | mount | 1522 | 1548 | 5000 | |
| ThemeProvider | mount | 1284 | 1265 | 5000 | |
| ThemeProvider | virtual-rerender | 623 | 646 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2140 | 2142 | 5000 | |
| Toggle | mount | 892 | 930 | 5000 | |
| buttonNative | mount | 151 | 145 | 5000 |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| TreeWith60ListItems.default | 220 | 193 | 1.14:1 |
| PortalMinimalPerf.default | 201 | 184 | 1.09:1 |
| AccordionMinimalPerf.default | 178 | 167 | 1.07:1 |
| AttachmentMinimalPerf.default | 198 | 185 | 1.07:1 |
| SegmentMinimalPerf.default | 400 | 375 | 1.07:1 |
| AvatarMinimalPerf.default | 228 | 216 | 1.06:1 |
| BoxMinimalPerf.default | 405 | 381 | 1.06:1 |
| FormMinimalPerf.default | 501 | 472 | 1.06:1 |
| GridMinimalPerf.default | 393 | 374 | 1.05:1 |
| LabelMinimalPerf.default | 456 | 434 | 1.05:1 |
| ListMinimalPerf.default | 580 | 551 | 1.05:1 |
| ReactionMinimalPerf.default | 449 | 428 | 1.05:1 |
| ChatDuplicateMessagesPerf.default | 340 | 327 | 1.04:1 |
| HeaderMinimalPerf.default | 425 | 408 | 1.04:1 |
| ListWith60ListItems.default | 745 | 716 | 1.04:1 |
| RefMinimalPerf.default | 254 | 244 | 1.04:1 |
| AttachmentSlotsPerf.default | 1232 | 1199 | 1.03:1 |
| CardMinimalPerf.default | 652 | 631 | 1.03:1 |
| ChatMinimalPerf.default | 742 | 721 | 1.03:1 |
| ChatWithPopoverPerf.default | 452 | 438 | 1.03:1 |
| DividerMinimalPerf.default | 419 | 408 | 1.03:1 |
| ToolbarMinimalPerf.default | 1069 | 1042 | 1.03:1 |
| AnimationMinimalPerf.default | 459 | 451 | 1.02:1 |
| ButtonMinimalPerf.default | 205 | 201 | 1.02:1 |
| DatepickerMinimalPerf.default | 6029 | 5938 | 1.02:1 |
| DialogMinimalPerf.default | 814 | 800 | 1.02:1 |
| FlexMinimalPerf.default | 324 | 318 | 1.02:1 |
| ItemLayoutMinimalPerf.default | 1379 | 1351 | 1.02:1 |
| LayoutMinimalPerf.default | 414 | 405 | 1.02:1 |
| MenuMinimalPerf.default | 947 | 927 | 1.02:1 |
| RadioGroupMinimalPerf.default | 504 | 494 | 1.02:1 |
| SliderMinimalPerf.default | 1858 | 1826 | 1.02:1 |
| SplitButtonMinimalPerf.default | 4699 | 4610 | 1.02:1 |
| TableManyItemsPerf.default | 2182 | 2139 | 1.02:1 |
| TableMinimalPerf.default | 482 | 471 | 1.02:1 |
| TextAreaMinimalPerf.default | 607 | 594 | 1.02:1 |
| ButtonOverridesMissPerf.default | 1959 | 1936 | 1.01:1 |
| CheckboxMinimalPerf.default | 2959 | 2928 | 1.01:1 |
| DropdownMinimalPerf.default | 3375 | 3354 | 1.01:1 |
| ListNestedPerf.default | 632 | 627 | 1.01:1 |
| LoaderMinimalPerf.default | 761 | 757 | 1.01:1 |
| MenuButtonMinimalPerf.default | 1816 | 1794 | 1.01:1 |
| ProviderMinimalPerf.default | 1240 | 1224 | 1.01:1 |
| StatusMinimalPerf.default | 775 | 764 | 1.01:1 |
| TextMinimalPerf.default | 387 | 385 | 1.01:1 |
| TreeMinimalPerf.default | 891 | 884 | 1.01:1 |
| AlertMinimalPerf.default | 312 | 311 | 1:1 |
| HeaderSlotsPerf.default | 855 | 853 | 1:1 |
| ImageMinimalPerf.default | 446 | 447 | 1:1 |
| InputMinimalPerf.default | 1424 | 1421 | 1:1 |
| PopupMinimalPerf.default | 634 | 633 | 1:1 |
| ProviderMergeThemesPerf.default | 1829 | 1832 | 1:1 |
| CustomToolbarPrototype.default | 4408 | 4400 | 1:1 |
| CarouselMinimalPerf.default | 512 | 519 | 0.99:1 |
| DropdownManyItemsPerf.default | 778 | 787 | 0.99:1 |
| EmbedMinimalPerf.default | 4687 | 4725 | 0.99:1 |
| SkeletonMinimalPerf.default | 397 | 400 | 0.99:1 |
| TooltipMinimalPerf.default | 1121 | 1133 | 0.99:1 |
| IconMinimalPerf.default | 690 | 705 | 0.98:1 |
| RosterPerf.default | 1307 | 1350 | 0.97:1 |
| VideoMinimalPerf.default | 707 | 727 | 0.97:1 |
| ButtonSlotsPerf.default | 602 | 630 | 0.96:1 |
| ListCommonPerf.default | 708 | 737 | 0.96:1 |
| fontFamily: theme.fontFamilyBase, | ||
| background: theme.colorNeutralBackground1, |
There was a problem hiding this comment.
| fontFamily: theme.fontFamilyBase, | |
| background: theme.colorNeutralBackground1, |
This should not be required as FluentProvider injects these styles:
There was a problem hiding this comment.
Unfortunately the story area has already been styled with a specific hex colour value as a part of the redesign with a higher specificity.
There was a problem hiding this comment.
I can remove font family
There was a problem hiding this comment.
Unfortunately the story area has already been styled with a specific hex colour value as a part of the redesign with a higher specificity.
Is it required? Just curious for what purpose, may be we can use there is a different element for that scenario?
There was a problem hiding this comment.
It was a part of the work that was done in tandem with designers to make storybook 'look more Fluent' don't hve much more context apart from that
I can understand that we might want that panel to have a certain background so that's why I ended up applying the background specifically to the container for the story so theme switching would look sane in dark and HC mode
miroslavstastny
left a comment
There was a problem hiding this comment.
packages/react-storybook-addon/src/components/.gitkeep should be removed
The gitkeep is already removed @miroslavstastny |
| showPanel: true, | ||
| panelPosition: 'right', | ||
| theme, | ||
| toolbar: { |
There was a problem hiding this comment.
why is this needed ? --docs mode will remove those by default no?
There was a problem hiding this comment.
it's not necessary, it removes the current toolbar items which IMO are never used in the interal dev loop
There was a problem hiding this comment.
Removed this entire file
| import { THEME_ID } from '../constants'; | ||
| import { FluentGlobals, FluentStoryContext } from '../hooks'; | ||
|
|
||
| import { makeStyles } from '@fluentui/react-make-styles'; |
There was a problem hiding this comment.
can we remove need for make styles ? introducing custom styling solution in storybook addon is not a good idea in general.
There was a problem hiding this comment.
I'm not sure what 'in general' means here. The addon already uses FluentProvider as a dependency.
makeStyles is necessary in the FluentProvider since we need a background for stories that can be theme switched. (see #20346 (comment))
I've kept the make styles usage only for the decorator now and removed it from the toolbar
There was a problem hiding this comment.
I'm not sure what 'in general' means here.
it means that storybook has its own way of styling (emotion). to follow the API of addons unified solutions should be used instead of custom one especially for public facing addons.
There was a problem hiding this comment.
makeStyles is necessary in the FluentProvider since we need a background for stories that can be theme switched. (see #20346 (comment))
If I'm not wrong having this styled via emotion (storybook way) or inline would increase the specificity so there is no need for make-styles being used manually. What the Provider does under the hood is implementation detail and addon should not care about it - thus following generic addons styling solution. Obtaining the color value should be achievable directly from react-theme
There was a problem hiding this comment.
Ah ok that makes sense, I fetched the direct theme value and removed makestyles in this commit
|
CI is failing ^ |
) * experiment(storybook): Version picker Chromatic stores published storybooks with commits, we can simply build an azure function to keep the version -> commit mapping and integrate a picker into the docs page * Change files * use fluent menu * cleanup * add network fetch * use addon * remove old * update deps * update stypes * update md * remove export * chore(storybook): Add theme picker to storybook toolbar Adds the theme picker to the storybook toolbar for internal development. The global types are exported since they will be used for the docs page. * remove unnecessary changes * update change * remove font family style * pr suggestions * re-show the other items in the toolbar * revert export * update readme * remove usage of make-styles * update syncpack * update md
Pull request checklist
$ yarn changeDescription of changes
Adds the theme picker to the storybook toolbar for internal development. The current
withFluentProviderdecorator will be replaced with a new one that is controlled by the new toolbar picker that uses storybook global state.The global types are exported since they will be used for the docs page.
Kudos to @Hotell for his work in #19250 which this PR steals most of 💪💪
Focus areas to test
(optional)