Skip to content
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

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

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
0157f8f
implement withFluentVRTestVariants decorator
TristanWatanabe Oct 8, 2022
d00208f
update test to include new decorator
TristanWatanabe Oct 8, 2022
c797198
update typings
TristanWatanabe Oct 10, 2022
bda9ea4
fix: typing and update API
TristanWatanabe Oct 10, 2022
dcfcde3
Add usage of decorator to README
TristanWatanabe Oct 10, 2022
794bd7f
remove unneed peerDep
TristanWatanabe Oct 10, 2022
f306846
fix: typing
TristanWatanabe Oct 10, 2022
fb87708
Update API
TristanWatanabe Oct 10, 2022
7bf1873
revert additions to react-storybook package
TristanWatanabe Oct 12, 2022
ed168ee
delete withFluentVrTestVariants
TristanWatanabe Oct 12, 2022
d3765ac
remove withFluentVrTestVariant
TristanWatanabe Oct 12, 2022
1f26987
feat: withFluentProvider now accepts fluentTheme and dir
TristanWatanabe Oct 12, 2022
66c95ef
Add exported theme constants
TristanWatanabe Oct 12, 2022
ab22cd5
feat: add isVrTest parameter
TristanWatanabe Oct 13, 2022
060d696
Revert "Add usage of decorator to README"
TristanWatanabe Oct 13, 2022
18c3e6d
update ReadMe
TristanWatanabe Oct 13, 2022
14847d9
Add constants and FluentParameters to exports
TristanWatanabe Oct 13, 2022
20091d7
Update API
TristanWatanabe Oct 13, 2022
e840ba7
Revert "Update API"
TristanWatanabe Oct 13, 2022
21eba2e
Revert "fix: typing and update API"
TristanWatanabe Oct 13, 2022
81de523
feedback: don't export constants, export FluentParameter type
TristanWatanabe Oct 16, 2022
b5f3788
feedback: replace isVrTest param with more dynamic mode param
TristanWatanabe Oct 16, 2022
4842eb9
feedback: add parameters identity function and export
TristanWatanabe Oct 16, 2022
8317329
Update API
TristanWatanabe Oct 16, 2022
55836b6
Update README
TristanWatanabe Oct 16, 2022
1559515
Merge branch 'master' into add-VrTestVariants-decorator
TristanWatanabe Oct 16, 2022
d888f92
remove unintended prettier changes
TristanWatanabe Oct 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions packages/react-components/react-storybook-addon/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,34 @@ module.exports = {
- > 💡 this will run `prestorybook` script that compiles addon implementation with all of its direct dependencies that live within monorepo, so it can be consumed by local storybook

2. Every time you do any change to implementation, after you ran your local storybook you'll need to manually run `yarn workspace @fluentui/react-storybook-addon build` to reflect those changes

## Parameter Configuration

- 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. `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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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
};
  1. 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;
}

Copy link
Member Author

@TristanWatanabe TristanWatanabe Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

import { Button } from '@fluentui/react-components';

export const Button = () => <Button> Hello World </Button>;

export const ButtonDarkMode = {
render: Button,
parameters: { fluentTheme: WEB_DARK }, // story renders in Dark mode.
};

export const ButtonHighContrast = {
render: Button,
parameters: { fluentTheme: TEAMS_HIGH_CONTRAST, isVrTest: true }; // story renders in High Contrast mode without injected padding and background style.
}

export const ButtonRTL = {
render: Button,
parameters: { fluentTheme: WEB_LIGHT, dir: 'rtl', isVrTest: true}, // story renders in RTL, Web light mode and without injected padding and background style.
};

```
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
```ts

import { Args } from '@storybook/api';
import { Parameters as Parameters_2 } from '@storybook/api';
import { StoryContext } from '@storybook/addons';
import type { Theme } from '@fluentui/react-theme';

Expand All @@ -14,12 +15,33 @@ export interface FluentGlobals extends Args {
[THEME_ID]?: ThemeIds;
}

// @public
export interface FluentParameters extends Parameters_2 {
// (undocumented)
dir?: 'ltr' | 'rtl';
// (undocumented)
fluentTheme?: ThemeIds;
// (undocumented)
isVrTest?: boolean;
}

// @public (undocumented)
export interface FluentStoryContext extends StoryContext {
// (undocumented)
globals: FluentGlobals;
// (undocumented)
parameters: FluentParameters;
}

// @public (undocumented)
export const TEAMS_DARK = "teams-dark";

// @public (undocumented)
export const TEAMS_HIGH_CONTRAST = "teams-high-contrast";

// @public (undocumented)
export const TEAMS_LIGHT = "teams-light";

// @public (undocumented)
export const THEME_ID: "storybook/fluentui-react-addon/theme";

Expand Down Expand Up @@ -49,6 +71,12 @@ export const themes: readonly [{
readonly theme: Theme;
}];

// @public (undocumented)
export const WEB_DARK = "web-dark";

// @public (undocumented)
export const WEB_LIGHT = "web-light";

// (No @packageDocumentation comment for this package)

```
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import * as React from 'react';
import { IconButton, Icons, TooltipLinkList, WithTooltip } from '@storybook/components';
import { useParameter } from '@storybook/api';

import { ThemeIds, themes, defaultTheme } from '../theme';
import { THEME_ID } from '../constants';
import { useGlobals } from '../hooks';
import { useGlobals, FluentParameters } from '../hooks';

export interface ThemeSelectorItem {
id: string;
Expand Down Expand Up @@ -33,7 +34,9 @@ function createThemeItems(

export const ThemePicker = () => {
const [globals, updateGlobals] = useGlobals();
const selectedThemeId = globals[THEME_ID] ?? defaultTheme.id;
const fluentTheme: FluentParameters['fluentTheme'] = useParameter('fluentTheme');

const selectedThemeId = fluentTheme ? fluentTheme : globals[THEME_ID] ?? defaultTheme.id;
const selectedTheme = themes.find(entry => entry.id === selectedThemeId);

const isActive = selectedThemeId !== defaultTheme.id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,34 @@ import * as React from 'react';

import { FluentProvider } from '@fluentui/react-provider';
import { Theme } from '@fluentui/react-theme';

import { themes, defaultTheme } from '../theme';
import { themes, defaultTheme, ThemeIds } from '../theme';
import { THEME_ID } from '../constants';
import { FluentGlobals, FluentStoryContext } from '../hooks';

const findTheme = (themeId?: ThemeIds) => {
if (!themeId) {
return;
}
return themes.find(value => value.id === themeId);
};

const getActiveFluentTheme = (globals: FluentGlobals) => {
const selectedThemeId = globals[THEME_ID];
const { theme } = themes.find(value => value.id === selectedThemeId) ?? defaultTheme;
const { theme } = findTheme(selectedThemeId) ?? defaultTheme;

return { theme };
};

export const withFluentProvider = (StoryFn: () => JSX.Element, context: FluentStoryContext) => {
const { theme } = getActiveFluentTheme(context.globals);
const { globals, parameters } = context;
const { isVrTest } = parameters;
const globalTheme = getActiveFluentTheme(globals);
const paramTheme = findTheme(parameters.fluentTheme);
const { theme } = paramTheme ?? globalTheme;

return (
<FluentProvider theme={theme}>
<FluentExampleContainer theme={theme}>{StoryFn()}</FluentExampleContainer>
<FluentProvider theme={theme} dir={parameters.dir}>
{isVrTest ? StoryFn() : <FluentExampleContainer theme={theme}>{StoryFn()}</FluentExampleContainer>}
</FluentProvider>
);
};
Expand All @@ -28,5 +38,5 @@ const FluentExampleContainer: React.FC<{ theme: Theme }> = props => {
const { theme } = props;

const backgroundColor = theme.colorNeutralBackground2;
return <div style={{ padding: '48px 24px', backgroundColor: backgroundColor }}>{props.children}</div>;
return <div style={{ padding: '48px 24px', backgroundColor }}>{props.children}</div>;
};
14 changes: 12 additions & 2 deletions packages/react-components/react-storybook-addon/src/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
import { useGlobals as useStorybookGlobals, Args as StorybookArgs } from '@storybook/api';
import { useGlobals as useStorybookGlobals, Args as StorybookArgs, Parameters } from '@storybook/api';
import { StoryContext as StorybookContext } from '@storybook/addons';

import { THEME_ID } from './constants';
import { ThemeIds } from './theme';

export interface FluentStoryContext extends StorybookContext {
globals: FluentGlobals;
parameters: FluentParameters;
}

/**
* Extends the storybook globals object to include fluent specific propoerties
* Extends the storybook globals object to include fluent specific properties
*/
export interface FluentGlobals extends StorybookArgs {
[THEME_ID]?: ThemeIds;
}

/**
* Extends the storybook parameters object to include fluent specific properties
*/
export interface FluentParameters extends Parameters {
dir?: 'ltr' | 'rtl';
fluentTheme?: ThemeIds;
isVrTest?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

export function useGlobals(): [FluentGlobals, (newGlobals: FluentGlobals) => void] {
return useStorybookGlobals();
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export type { FluentGlobals, FluentStoryContext } from './hooks';
export type { FluentGlobals, FluentParameters, FluentStoryContext } from './hooks';
export type { ThemeIds } from './theme';
export { themes } from './theme';
export { themes, TEAMS_DARK, TEAMS_HIGH_CONTRAST, TEAMS_LIGHT, WEB_DARK, WEB_LIGHT } from './theme';
export { THEME_ID } from './constants';
16 changes: 11 additions & 5 deletions packages/react-components/react-storybook-addon/src/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,18 @@ import {

import type { Theme } from '@fluentui/react-theme';

export const WEB_LIGHT = 'web-light';
TristanWatanabe marked this conversation as resolved.
Show resolved Hide resolved
export const WEB_DARK = 'web-dark';
export const TEAMS_LIGHT = 'teams-light';
export const TEAMS_DARK = 'teams-dark';
export const TEAMS_HIGH_CONTRAST = 'teams-high-contrast';

export const themes = [
{ id: 'web-light', label: 'Web Light', theme: webLightTheme },
{ id: 'web-dark', label: 'Web Dark', theme: webDarkTheme },
{ id: 'teams-light', label: 'Teams Light', theme: teamsLightTheme },
{ id: 'teams-dark', label: 'Teams Dark', theme: teamsDarkTheme },
{ id: 'teams-high-contrast', label: 'Teams High Contrast', theme: teamsHighContrastTheme },
{ id: WEB_LIGHT, label: 'Web Light', theme: webLightTheme },
{ id: WEB_DARK, label: 'Web Dark', theme: webDarkTheme },
{ id: TEAMS_LIGHT, label: 'Teams Light', theme: teamsLightTheme },
{ id: TEAMS_DARK, label: 'Teams Dark', theme: teamsDarkTheme },
{ id: TEAMS_HIGH_CONTRAST, label: 'Teams High Contrast', theme: teamsHighContrastTheme },
] as const;

export const defaultTheme = themes[0];
Expand Down