RFC(storybook): Structural proposals to organize stories for E2E by ling1726 · Pull Request #18895 · microsoft/fluentui

ling1726 · 2021-07-11T17:02:02Z

This PR builds on a previous RFC and proposes an organizational
structure for colocated storybooks and an internal story system for E2E
testing of edge cases whose stories should not be publicly avaialble.

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ yarn change

Description of changes

(give an overview)

Focus areas to test

(optional)

This PR builds on a previous RFC and proposes an organizational structure for colocated storybooks and an internal story system for E2E testing of edge cases whose stories should not be publicly avaialble.

size-auditor · 2021-07-11T17:12:45Z

Asset size changes

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

Baseline commit: f8578ee238037bb8af86582d7f75659ff0bc8ebd (build)

codesandbox-ci · 2021-07-11T17:19:55Z

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

Sandbox	Source
Fluent UI Button	Configuration
codesandbox-react-template	Configuration
codesandbox-react-northstar-template	Configuration

fabricteam · 2021-07-11T17:26:06Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	748	743	5000
BaseButton	mount	835	853	5000
Breadcrumb	mount	2500	2466	1000
ButtonNext	mount	505	503	5000
Checkbox	mount	1455	1405	5000
CheckboxBase	mount	1207	1224	5000
ChoiceGroup	mount	4500	4480	5000
ComboBox	mount	986	928	1000
CommandBar	mount	9532	9666	1000
ContextualMenu	mount	5910	5901	1000
DefaultButton	mount	1076	1060	5000
DetailsRow	mount	3523	3471	5000
DetailsRowFast	mount	3539	3537	5000
DetailsRowNoStyles	mount	3317	3329	5000
Dialog	mount	2013	2055	1000
DocumentCardTitle	mount	133	139	1000
Dropdown	mount	3062	3056	5000
FluentProviderNext	mount	6946	6903	5000
FocusTrapZone	mount	1725	1716	5000
FocusZone	mount	1701	1708	5000
IconButton	mount	1626	1672	5000
Label	mount	312	330	5000
Layer	mount	1667	1636	5000
Link	mount	430	455	5000
MakeStyles	mount	1687	1707	50000
MenuButton	mount	1374	1386	5000
MessageBar	mount	1873	1914	5000
Nav	mount	3023	3031	1000
OverflowSet	mount	961	979	5000
Panel	mount	1945	1971	1000
Persona	mount	773	762	1000
Pivot	mount	1348	1331	1000
PrimaryButton	mount	1232	1202	5000
Rating	mount	7236	7277	5000
SearchBox	mount	1290	1247	5000
Shimmer	mount	2405	2426	5000
Slider	mount	1817	1826	5000
SpinButton	mount	4738	4678	5000
Spinner	mount	404	381	5000
SplitButton	mount	2945	2971	5000
Stack	mount	473	450	5000
StackWithIntrinsicChildren	mount	1455	1464	5000
StackWithTextChildren	mount	4267	4255	5000
SwatchColorPicker	mount	9786	9761	5000
Tabs	mount	1311	1305	1000
TagPicker	mount	2310	2288	5000
TeachingBubble	mount	11298	11212	5000
Text	mount	400	388	5000
TextField	mount	1302	1277	5000
ThemeProvider	mount	1106	1117	5000
ThemeProvider	virtual-rerender	565	568	5000
Toggle	mount	759	766	5000
buttonNative	mount	119	107	5000

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

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
ButtonMinimalPerf.default	160	154	1.04:1
ButtonSlotsPerf.default	543	524	1.04:1
HeaderMinimalPerf.default	340	328	1.04:1
SkeletonMinimalPerf.default	334	322	1.04:1
DropdownManyItemsPerf.default	651	630	1.03:1
LayoutMinimalPerf.default	353	344	1.03:1
IconMinimalPerf.default	594	578	1.03:1
AttachmentSlotsPerf.default	1013	989	1.02:1
DatepickerMinimalPerf.default	5156	5073	1.02:1
GridMinimalPerf.default	323	317	1.02:1
ImageMinimalPerf.default	346	338	1.02:1
MenuMinimalPerf.default	806	791	1.02:1
MenuButtonMinimalPerf.default	1587	1563	1.02:1
PopupMinimalPerf.default	574	563	1.02:1
ProviderMinimalPerf.default	953	932	1.02:1
RefMinimalPerf.default	225	220	1.02:1
StatusMinimalPerf.default	637	625	1.02:1
ToolbarMinimalPerf.default	893	877	1.02:1
CarouselMinimalPerf.default	435	431	1.01:1
LabelMinimalPerf.default	366	362	1.01:1
ListCommonPerf.default	596	589	1.01:1
ListWith60ListItems.default	597	593	1.01:1
PortalMinimalPerf.default	168	167	1.01:1
ProviderMergeThemesPerf.default	1602	1584	1.01:1
TextAreaMinimalPerf.default	470	465	1.01:1
CustomToolbarPrototype.default	3669	3627	1.01:1
AlertMinimalPerf.default	258	259	1:1
ButtonOverridesMissPerf.default	1625	1628	1:1
ChatMinimalPerf.default	615	612	1:1
CheckboxMinimalPerf.default	2615	2604	1:1
DropdownMinimalPerf.default	2944	2940	1:1
EmbedMinimalPerf.default	3892	3909	1:1
FormMinimalPerf.default	377	377	1:1
HeaderSlotsPerf.default	706	709	1:1
InputMinimalPerf.default	1197	1196	1:1
ItemLayoutMinimalPerf.default	1145	1143	1:1
LoaderMinimalPerf.default	656	659	1:1
RadioGroupMinimalPerf.default	418	418	1:1
ReactionMinimalPerf.default	344	345	1:1
SliderMinimalPerf.default	1495	1489	1:1
SplitButtonMinimalPerf.default	3562	3573	1:1
TableManyItemsPerf.default	1765	1764	1:1
TooltipMinimalPerf.default	939	937	1:1
AttachmentMinimalPerf.default	146	148	0.99:1
BoxMinimalPerf.default	328	332	0.99:1
CardMinimalPerf.default	518	522	0.99:1
ListMinimalPerf.default	476	479	0.99:1
ListNestedPerf.default	507	513	0.99:1
TreeMinimalPerf.default	737	748	0.99:1
TreeWith60ListItems.default	157	158	0.99:1
AnimationMinimalPerf.default	389	398	0.98:1
DividerMinimalPerf.default	338	344	0.98:1
VideoMinimalPerf.default	578	588	0.98:1
AccordionMinimalPerf.default	134	138	0.97:1
ChatWithPopoverPerf.default	329	338	0.97:1
DialogMinimalPerf.default	711	731	0.97:1
SegmentMinimalPerf.default	313	323	0.97:1
TableMinimalPerf.default	376	387	0.97:1
TextMinimalPerf.default	320	331	0.97:1
FlexMinimalPerf.default	266	276	0.96:1
AvatarMinimalPerf.default	178	189	0.94:1
ChatDuplicateMessagesPerf.default	271	287	0.94:1
RosterPerf.default	1067	1169	0.91:1

ling1726 · 2021-07-12T07:56:10Z

/azp run

azure-pipelines · 2021-07-12T07:56:36Z

Azure Pipelines successfully started running 3 pipeline(s).

Hotell · 2021-07-19T14:06:24Z

+> This may change in the future so the measures proposed in this section might no longer be relevant
+> if this assumption is no longer true.

-### 6. how to properly annotate stories with TS metadata to get the best DX possible


pls make sure to run npx markdown-toc -i rfcs/convergence/authoring-stories.md as you changed chapter ordering

Hotell · 2021-07-19T14:19:48Z

 ### 7. how structure stories in the storybook nav tree

-TBA
+> This section assumes that the storybook stories forms the core documentation of Fluent converged.


Can we remove this "pre-requirement" -> I don't think this is accurate as these style guides should be adhered to no matter if we use SB as our docs or not. Same applies for previous chapter in this PR.

Well we no longer need any of the proposals in this PR if that's the case, because we would never be showing consumers strange internal stories to test edge cases

not sure I follow

Hotell · 2021-07-19T14:23:35Z

+
+We propose to write an NX workspace generator to setup the e2e testing folder structure and dependencies for each package. The process is quite simple currently but still needs to be done manually.
+
+How to write E2E tests is out of the scope of the RFC. Testing is encouraged to use publicly viewable stories as much as possible. However there will be edge cases and scenarios that will be tested which require stories that are accessible to the E2E test runner but not viewable by public consumers. We provide a proposal to handle this in a later section in this RFC.


can we add link to that chapter here ?

We provide a [proposal to handle this scenario](#Internal_stories) in a later section in this RFC

Hotell · 2021-07-19T14:45:04Z

+`react-accordion` and `react-popover`. The purposes of E2E tests currently is to run internaction tests against the browser.
+Visual regression tests are handled by screener in `app-vrtests` and is out of the scope of this RFC.
+
+We propose to write an NX workspace generator to setup the e2e testing folder structure and dependencies for each package. The process is quite simple currently but still needs to be done manually.


as we discussed back in the day, this is already build in in nx.

Only difference with "current" setup is more opinionated differentiation of e2e suites as separate project.

What does that mean:

meaning we would have for each package a separate <project-name>-e2e project that would contain solely e2e scenarios

This has various benefits:

already existing generators that understand AST by nx core team (less work on our side)

no direct imports allowed from implementation, covered by tooling (direct imports defeat the purpose of proper e2e scenarios)

faster pipelines/scope as *-e2e app would not trigger implementation pipeline, only the other way (because e2e has it as implicit dependency / not direct one )

We can move to separate project/proposal, in another proposal since it changes more drastically the architecture of the repo. I'm just trying to build on what is already there.

it changes more drastically the architecture of the repo

thats a bit exaggerated don't you think ? :) from my POV its a straightforward change, but sure let's address this in separate RFC/proposal

Hotell · 2021-07-19T15:36:23Z

+
+export const MenuTabstopsInternal = () => {
+  // story
+};


just for clarification, can we mention (add to the example if needed) that those file will still need to follow storybooks CSF, thus containing default export?

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

fabricteam · 2021-07-19T16:47:11Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-divider` Divider	`15.889 kB` `5.747 kB`
`react-image` Image	`10.642 kB` `4.264 kB`
`react-make-styles` makeStaticStyles (runtime)	`7.59 kB` `3.321 kB`
`react-make-styles` makeStyles + mergeClasses (runtime)	`22.135 kB` `8.356 kB`
`react-make-styles` makeStyles + mergeClasses (build time)	`2.557 kB` `1.202 kB`
`react-menu` Menu - Default	`113.947 kB` `34.389 kB`
`react-menu` Menu - Selectable	`115.959 kB` `34.649 kB`
`react-popover` Popover	`140.938 kB` `41.968 kB`
`react-theme` Teams: all themes	`31.935 kB` `6.49 kB`
`react-theme` Teams: Light theme	`19.527 kB` `5.504 kB`

_{🤖 This report was generated against f8578ee238037bb8af86582d7f75659ff0bc8ebd}

PeterDraex

I'd suggest moving point 6 and the first part of point 7 to its own PR for future reference. Then we could merge point 5 and 7/Internal stories.

PeterDraex · 2021-07-21T09:49:11Z

+The filename extension `.internal.` is used for IDE searchability and codebase readability.
+
+The naming convention of the story simply adds the `Internal` keyword to the Pascal case story name. This will match the
+filename. More importantly the generated id will contain `menu-tabstops-internal`.


Suggested change

filename. More importantly the generated id will contain `menu-tabstops-internal`.

filename. More importantly the generated id will end with `internal`. For example the whole id could be `menu-tabstops-internal`.

PeterDraex · 2021-07-21T11:55:45Z

+
+### 6. dissecting big story files into smaller ones
+
+For easier maintainability, each story should be in its own file. Common utilities can be shared in their own modules.


I can see that it’s not great to scroll through a really long story file which contains many long stories. However, separating each story to its own file has the following disadvantages:

It creates overhead of repeating imports/default export for each file.

For simple components, this can be overkill. Even Button story file with 11 stories, looks OK to me, all trade offs being considered.

For proper functioning of Docs mode, it requires repeating of titles, which breaks CSF (titles aren’t supposed to be repeated). Given that CSF is the recommended way of writing Stories, going against it is setting ourselves up for issues.

Also, we’d need to verify if there’d still be a way to use component level Storybook configuration with this set up, and if not, consider if we can get by without it.

Alright, so your proposal is that we keep one single story file for each component ?

Probably. I think it’s best to have one story file, but we could still import story functions from other files (that wouldn’t have the .stories.tsx extension). I need like one more week to try it out and formulate a proposal. Do you see any downsides to this approach right now?

As you say Button has many stories, however in the Menu case there are equally as many stories but the stories are longer and more complicated and soe stories need the use of components outside the story export itself.

Menu.stories.tsx contains 11 stories in a file that is 400 lines long, we can keep it, but it might continue to grow especially if we want to test stories that are not part of the public documentation

ling1726 · 2021-07-30T15:37:10Z

Closing in favour of #19206

Will add the E2E internal stories section once that PR has been merged... FYI @PeterDraex

Adds relevant proposal from microsoft#18895 for hiding certain stories only required for internal e2e testing to our storybook.

* RFC: Internal stories for E2E testing Adds relevant proposal from #18895 for hiding certain stories only required for internal e2e testing to our storybook. * update * Update authoring-stories.md * Update authoring-stories.md * Update authoring-stories.md

RFC(storybook): Structural proposals to organize stories for E2E

b135c1b

This PR builds on a previous RFC and proposes an organizational structure for colocated storybooks and an internal story system for E2E testing of edge cases whose stories should not be publicly avaialble.

msft-fluent-ui-bot assigned ling1726 Jul 11, 2021

ling1726 added the Type: RFC Request for Feedback label Jul 11, 2021

ling1726 requested review from a team July 11, 2021 17:02

Hotell reviewed Jul 19, 2021

View reviewed changes

ling1726 and others added 5 commits July 19, 2021 18:27

Update rfcs/convergence/authoring-stories.md

2656a04

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

Update rfcs/convergence/authoring-stories.md

7b9830d

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

Update rfcs/convergence/authoring-stories.md

6cf4092

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

Update rfcs/convergence/authoring-stories.md

f01eea0

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

Update rfcs/convergence/authoring-stories.md

011a4f5

Co-authored-by: Martin Hochel <hochelmartin@gmail.com>

PeterDraex suggested changes Jul 23, 2021

View reviewed changes

msft-fluent-ui-bot added Needs: Author Feedback and removed Needs: Author Feedback labels Jul 23, 2021

theerebuss added this to the July Project Cycle Q2 2021 milestone Jul 26, 2021

PeterDraex mentioned this pull request Jul 30, 2021

RFC(storybook): conventions for story organization, naming and design #19206

Merged

ling1726 closed this Jul 30, 2021

ling1726 added a commit to ling1726/fluentui that referenced this pull request Aug 10, 2021

RFC: Internal stories for E2E testing

b714355

Adds relevant proposal from microsoft#18895 for hiding certain stories only required for internal e2e testing to our storybook.

ling1726 mentioned this pull request Aug 10, 2021

RFC: Internal stories for E2E testing #19333

Merged


		We propose to write an NX workspace generator to setup the e2e testing folder structure and dependencies for each package. The process is quite simple currently but still needs to be done manually.

		How to write E2E tests is out of the scope of the RFC. Testing is encouraged to use publicly viewable stories as much as possible. However there will be edge cases and scenarios that will be tested which require stories that are accessible to the E2E test runner but not viewable by public consumers. We provide a proposal to handle this in a later section in this RFC.

	filename. More importantly the generated id will contain `menu-tabstops-internal`.
	filename. More importantly the generated id will end with `internal`. For example the whole id could be `menu-tabstops-internal`.


		### 6. dissecting big story files into smaller ones

		For easier maintainability, each story should be in its own file. Common utilities can be shared in their own modules.

Uh oh!

Conversation

ling1726 commented Jul 11, 2021

Pull request checklist

Description of changes

Focus areas to test

Uh oh!

size-auditor Bot commented Jul 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

codesandbox-ci Bot commented Jul 11, 2021

Uh oh!

fabricteam commented Jul 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react-northstar)

Uh oh!

ling1726 commented Jul 12, 2021

Uh oh!

azure-pipelines Bot commented Jul 12, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ling1726 Jul 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fabricteam commented Jul 19, 2021

📊 Bundle size report

Uh oh!

PeterDraex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ling1726 Jul 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ling1726 commented Jul 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

size-auditor Bot commented Jul 11, 2021 •

edited

Loading

fabricteam commented Jul 11, 2021 •

edited

Loading

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

ling1726 Jul 19, 2021 •

edited

Loading

ling1726 Jul 26, 2021 •

edited

Loading