feat(react-storybook-addon): make withFluentProvider decorator configurable to be used for VR tests #25162

TristanWatanabe · 2022-10-10T22:22:27Z

Changes:

Pulled out of chore(vr-tests-v9): Convert Button VR tests to CSF #25108. This PR updates the withFluentProvider decorator and makes it configurable via story parameters to be usable when writing different Visual Regression test variants in v9. This does NOT change any default behavior of the addon, it simply allows it to be configurable for VR tests.
This PR is in preparation for migrating v9 VR tests to Component Story Format (CSF).
We extended storybook's storiesOf .add method (called .addStory) to automatically include RTL, Dark Mode and High Contrast variants via the setAddon method in the vr-tests-react-components package. setAddon is already deprecated and set to be removed in storybook 7.0 and there's no real 1-to-1 replacement for it. This updated decorator implementation aims to replace that functionality and will add the ability to pass a fluentTheme parameter to a story which will allow it to render as either Dark Mode, High Contrast (and more) and also adds a dir parameter which can receive an rtl value to render a story in RTL mode.

Parameter Options:

Three custom optional parameters can be set to alter behavior of the addon
1. dir - determines whether to render story in ltr or rtl mode. Default is undefined.
2. fluentTheme - determines whether to render story theme in web-light, web-dark, teams-high-contrast, teams-dark, or teams-light. Setting this parameter will disable ability to dynamically change the theme within story canvas or doc.
3. mode - when set to vr-test, this removes injected padding and background theme that's automatically applied from rendered story. Default is default.

Example Usage:

import { FluentParameters, parameters } from '@fluentui/react-storybook-addon';
import { Button } from '@fluentui/react-components';
export const Button = () => <Button> Hello World </Button>;
export const ButtonDarkMode = {
  render: Button,
  parameters: { fluentTheme: 'web-dark' } as FluentParameters, // story renders in Dark mode.
};
export const ButtonHighContrast = {
  render: Button,
  parameters: { fluentTheme: 'teams-high-contrast', mode: 'vr-test' } as FluentParameters; // story renders in High Contrast mode without injected padding and background style.
}
export const ButtonRTL = {
  render: Button,
  // parameters identity function will have all TS type annotations built in for intellisense.
  parameters: parameters({ fluentTheme: 'web-light', dir: 'rtl', mode: 'vr-test'}), // story renders in RTL, Web light mode and without injected padding and background style.
};

Issues:

Part of #25078

fabricteam · 2022-10-10T22:28:47Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-components` react-components: Button, FluentProvider & webLightTheme	`62.94 kB` `17.663 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`189.603 kB` `52.909 kB`
`react-components` react-components: FluentProvider & webLightTheme	`33.446 kB` `11.033 kB`
`react-portal-compat` PortalCompatProvider	`5.857 kB` `1.978 kB`

_{🤖 This report was generated against 59cc34eefb165b3a052da500465178b109f3df01}

codesandbox-ci · 2022-10-10T22:34:06Z

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 d888f92:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

size-auditor · 2022-10-10T22:34:46Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 59cc34eefb165b3a052da500465178b109f3df01 (build)

packages/react-components/react-storybook/src/decorators/withFluentVrTestVariants.tsx

ling1726 · 2022-10-11T06:50:42Z

packages/react-components/react-storybook/src/decorators/withFluentVrTestVariants.tsx

+      return <FluentProvider theme={teamsHighContrastTheme}>{storyFn(context)}</FluentProvider>;
+    }
+
+    return storyFn(context);


It seems that we have this new decorator that configures the provider but also withFluentProvider that is not configurable. Then let's just have one withFluentProvider that is configurable ?

^ great call, lets do it ! #DRY

Yeah I considered doing this but as it stands, the withFluentProvider decorator doesn’t just wrap a story function in FluentProvider, but it also wraps it in a container with custom styling. I wanted to avoid a bunch of Screener change churn that this would cause which is why I opted for a separate decorator.

in that case we could remove the custom styling into its own decorator ?

I wanted to avoid a bunch of Screener change churn

wondering what churn are you referring to ?

Since an extra div with styling is essentially added to each story by the withFluentProvider decorator, when each story is converted to CSF , Screener will detect each one as "Changed". I was trying to avoid a bunch of unnecessary Screener changes when I refactor to CSF to simplify the review process and to not introduce random screener issues

in that case we could remove the custom styling into its own decorator ?

With the way it's implemented right now, that would just seem redundant? Even if moved as a separate decorator, it would still need a FluentProvider regardless.

export const withFluentProvider = (StoryFn: () => JSX.Element, context: FluentStoryContext) => { const { theme } = getActiveFluentTheme(context.globals); return ( <FluentProvider theme={theme}> <FluentExampleContainer theme={theme}>{StoryFn()}</FluentExampleContainer> </FluentProvider> ); };

I'm not opposed to just reconfiguring the withFluentProvider so that it can accept theme and dir parameters and any screener changes (it adds padding and theme background) can just be communicated in the PR details - thoughts?

I wonder if we could utilize the storybook context or globals a bit more, withFluentProvider could maybe have like mode configuration where the VR tests would configure a mode that would not inject padding and theme background ?

ling1726 · 2022-10-11T06:57:45Z

We have a react-storybook and react-storybook-addon package that both have decorators that add a fluent provider. Generally we use the addon for v9 related things, and since the VR tests are decoupled from v8 I don't see a reason to keep v9 internals in the react-storybook package.

The withFluentProvider in react-storybook-addon is what we use in the docsite storybook and also doesn't use makeDecorator (which is no longer publicly documented AFAIK`

packages/react-components/react-storybook/src/decorators/withFluentVrTestVariants.tsx

packages/react-components/react-storybook/README.md

packages/react-components/react-storybook/src/index.ts

packages/react-components/react-storybook/README.md

This reverts commit dcfcde3.

This reverts commit fb87708.

This reverts commit bda9ea4.

fabricteam · 2022-10-14T01:16:04Z

Perf Analysis (`@fluentui/react-components`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	1316	1320	5000
Button	mount	944	947	5000
FluentProvider	mount	1602	1572	5000
FluentProviderWithTheme	mount	632	635	10
FluentProviderWithTheme	virtual-rerender	591	598	10
FluentProviderWithTheme	virtual-rerender-with-unmount	635	620	10
MakeStyles	mount	1814	1884	50000
SpinButton	mount	2526	2538	5000

packages/react-components/react-storybook/src/index.ts

Hotell · 2022-10-18T15:51:43Z

packages/react-components/react-storybook-addon/README.md

+  3. `isVrTest` - when set to `true`, this removes injected padding and background theme that's automatically applied from rendered story.
+
+```js
+import { TEAMS_HIGH_CONTRAST, WEB_DARK, WEB_LIGHT } from '@fluentui/react-storybook-addon';


suggestion: hmm best API is no API or very limited API surface 🙌.

we don't need these tokens for sake of setting fluent theme.

We can instead leverage one or both of the following approaches:

export only type that consumer can use to get proper intelissense for setting parameters

import type {Parameters} from '@fluentui/react-storybook-addon'; export const ButtonDarkMode = { render: Button, parameters: { fluentTheme: 'web-dark' } as Parameters };

export function that will provide all the intellisense without need to do any manual work

import {parameters} from '@fluentui/react-storybook-addon'; export const ButtonDarkMode = { render: Button, // parameters function has all the proper TS annotations so we will get top DX parameters: parameters({ fluentTheme: 'web-dark' }) };

// this will be basically an identity function export function parameters(options:FluentAddonParameters){ // possibly process default values here return options; }

These two options combined to provide TS typing via intellisense are definitely much better solutions than constant tokens! I've made the change to add a parameters function and have removed the constants in favor of exporting a FluentParameters type.

Hotell · 2022-10-18T15:56:12Z

packages/react-components/react-storybook-addon/src/hooks.ts

+export interface FluentParameters extends Parameters {
+  dir?: 'ltr' | 'rtl';
+  fluentTheme?: ThemeIds;
+  isVrTest?: boolean;


suggestion: boolean flags doesn't scale well. what about

-isVrTest?: boolean; +context?: 'vr-test' | 'default'

replaced with mode flag instead to prevent confusion with storybook context: b5f3788

packages/react-components/react-storybook-addon/src/theme.ts

Hotell

awesomeness 🤩

…urable to be used for VR tests (microsoft#25162)

TristanWatanabe added 6 commits October 8, 2022 09:01

implement withFluentVRTestVariants decorator

0157f8f

update test to include new decorator

d00208f

update typings

c797198

fix: typing and update API

bda9ea4

Add usage of decorator to README

dcfcde3

remove unneed peerDep

794bd7f

TristanWatanabe self-assigned this Oct 10, 2022

github-actions bot added this to the October Project Cycle Q4 2022 milestone Oct 10, 2022

TristanWatanabe added 2 commits October 10, 2022 16:03

fix: typing

f306846

Update API

fb87708

TristanWatanabe marked this pull request as ready for review October 10, 2022 23:26

TristanWatanabe requested review from a team as code owners October 10, 2022 23:26

ling1726 reviewed Oct 11, 2022

View reviewed changes

packages/react-components/react-storybook/src/decorators/withFluentVrTestVariants.tsx Outdated Show resolved Hide resolved

ling1726 reviewed Oct 11, 2022

View reviewed changes

Hotell reviewed Oct 11, 2022

View reviewed changes

TristanWatanabe added 10 commits October 12, 2022 11:42

revert additions to react-storybook package

7bf1873

delete withFluentVrTestVariants

ed168ee

remove withFluentVrTestVariant

d3765ac

feat: withFluentProvider now accepts fluentTheme and dir

1f26987

Add exported theme constants

66c95ef

feat: add isVrTest parameter

ab22cd5

Revert "Add usage of decorator to README"

060d696

This reverts commit dcfcde3.

update ReadMe

18c3e6d

Add constants and FluentParameters to exports

14847d9

Update API

20091d7

TristanWatanabe added 2 commits October 13, 2022 03:56

Revert "Update API"

e840ba7

This reverts commit fb87708.

Revert "fix: typing and update API"

21eba2e

This reverts commit bda9ea4.

TristanWatanabe changed the title ~~feat(react-storybook): add withFluentVRTestVariants decorator~~ feat(react-storybook-addon): make withFluentProvider decorator configurable to be used for VR tests Oct 14, 2022

TristanWatanabe requested review from ling1726 and Hotell October 14, 2022 01:22

ling1726 approved these changes Oct 14, 2022

View reviewed changes

TristanWatanabe added 6 commits October 16, 2022 04:49

feedback: don't export constants, export FluentParameter type

81de523

feedback: replace isVrTest param with more dynamic mode param

b5f3788

feedback: add parameters identity function and export

4842eb9

Update API

8317329

Update README

55836b6

Merge branch 'master' into add-VrTestVariants-decorator

1559515

Hotell reviewed Oct 18, 2022

View reviewed changes

packages/react-components/react-storybook-addon/src/theme.ts Outdated Show resolved Hide resolved

TristanWatanabe requested a review from Hotell October 19, 2022 21:45

TristanWatanabe requested review from a team and sopranopillow as code owners October 20, 2022 17:29

remove unintended prettier changes

d888f92

TristanWatanabe removed request for a team and sopranopillow October 20, 2022 17:30

This was referenced Oct 20, 2022

chore(vr-tests-v9): Convert Button VR tests to CSF #25108

Merged

[DO NOT MERGE Testing Screener] chore(vr-tests-v9): Convert v9 VR stories to CSF #25345

Closed

Hotell approved these changes Oct 25, 2022

View reviewed changes

TristanWatanabe merged commit ca05778 into microsoft:master Oct 25, 2022

TristanWatanabe deleted the add-VrTestVariants-decorator branch October 25, 2022 16:15

TristanWatanabe mentioned this pull request Oct 25, 2022

[Epic]: Visual Regression Tool Migration #25078

Closed

27 tasks

NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022

feat(react-storybook-addon): make withFluentProvider decorator config…

5dca84e

…urable to be used for VR tests (microsoft#25162)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(react-storybook-addon): make withFluentProvider decorator configurable to be used for VR tests #25162

feat(react-storybook-addon): make withFluentProvider decorator configurable to be used for VR tests #25162

TristanWatanabe commented Oct 10, 2022 •

edited

Loading

fabricteam commented Oct 10, 2022 •

edited

Loading

codesandbox-ci bot commented Oct 10, 2022 •

edited

Loading

size-auditor bot commented Oct 10, 2022 •

edited

Loading

ling1726 Oct 11, 2022 •

edited

Loading

Hotell Oct 11, 2022

TristanWatanabe Oct 12, 2022

ling1726 Oct 12, 2022

Hotell Oct 12, 2022

TristanWatanabe Oct 12, 2022 •

edited

Loading

TristanWatanabe Oct 12, 2022

TristanWatanabe Oct 12, 2022

ling1726 Oct 13, 2022

ling1726 commented Oct 11, 2022

fabricteam commented Oct 14, 2022 •

edited

Loading

Hotell Oct 18, 2022

TristanWatanabe Oct 19, 2022 •

edited

Loading

Hotell Oct 18, 2022

TristanWatanabe Oct 19, 2022

Hotell left a comment

feat(react-storybook-addon): make withFluentProvider decorator configurable to be used for VR tests #25162

feat(react-storybook-addon): make withFluentProvider decorator configurable to be used for VR tests #25162

Conversation

TristanWatanabe commented Oct 10, 2022 • edited Loading

Changes:

Parameter Options:

Example Usage:

Issues:

fabricteam commented Oct 10, 2022 • edited Loading

📊 Bundle size report

codesandbox-ci bot commented Oct 10, 2022 • edited Loading

size-auditor bot commented Oct 10, 2022 • edited Loading

Asset size changes

ling1726 Oct 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TristanWatanabe Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ling1726 commented Oct 11, 2022

fabricteam commented Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

TristanWatanabe Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hotell left a comment

Choose a reason for hiding this comment

TristanWatanabe commented Oct 10, 2022 •

edited

Loading

fabricteam commented Oct 10, 2022 •

edited

Loading

codesandbox-ci bot commented Oct 10, 2022 •

edited

Loading

size-auditor bot commented Oct 10, 2022 •

edited

Loading

ling1726 Oct 11, 2022 •

edited

Loading

TristanWatanabe Oct 12, 2022 •

edited

Loading

fabricteam commented Oct 14, 2022 •

edited

Loading

TristanWatanabe Oct 19, 2022 •

edited

Loading