RFC: Docs Component Anatomy & Audit #28326
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 584 | 605 | 5000 | |
| Button | mount | 291 | 293 | 5000 | |
| Field | mount | 1084 | 1044 | 5000 | |
| FluentProvider | mount | 672 | 682 | 5000 | |
| FluentProviderWithTheme | mount | 79 | 85 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 66 | 72 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 75 | 71 | 10 | |
| InfoButton | mount | 17 | 10 | 5000 | |
| MakeStyles | mount | 868 | 879 | 50000 | |
| Persona | mount | 1645 | 1627 | 5000 | |
| SpinButton | mount | 1306 | 1281 | 5000 |
|
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 7719ab1:
|
Collaborator
📊 Bundle size report🤖 This report was generated against 1d1aa7cc05cf1bee98e2dbebdd9a5040544379f1 |
Asset size changes
Baseline commit: 915acd693ae5c9ad8995d0cb952e804e9fd3e120 (build) |
Collaborator
🕵 fluentuiv9 No visual regressions between this PR and main |
levithomason
force-pushed
the
lethoma/docs-anatomy
branch
from
cdaf965 to
d01b355
Compare
Collaborator
🕵 fluentuiv9 No visual regressions between this PR and main |
levithomason
force-pushed
the
lethoma/docs-anatomy
branch
from
d01b355 to
57c126f
Compare
ValentinaKozlova
requested review from
tudorpopams
and removed request for
tudorpopams
levithomason
changed the title
levithomason
force-pushed
the
lethoma/docs-anatomy
branch
from
4e6a4d7 to
12e9669
Compare
Contributor
We have conformance tests for component class names which align with component anatomy so I think we do care about consistency and we can automate it, though this analysis shows we have room to improve on this front 🙂 |
levithomason
force-pushed
the
lethoma/docs-anatomy
branch
from
ed69458 to
baea991
Compare
Member
|
Closing for housekeeping 🏡 Feel free to re-open 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduction
This PR introduces an automatic anatomy highlighter for stories. It is a prototype to demonstrate the usefulness only.
Features:
Usefulness:
Authors: @levithomason @miroslavstastny
💬 Discussions
Integration
What is the best way to integrate this capability?
Consistency
The audit revealed mostly inconsistencies in:
We should answer:
We propose yes to all of the above as it is necessary to enable 2nd layer tooling and assistance, such as the anatomy viewer. Consistency is also required for any future design tools, token explorers, etc. which would sit on top of our patterns.
Composition Patterns
We should reach a conclusion on how to properly compose custom components.
📖 Collection Component Docs
There are several inconsistencies and unresolved patterns with collection components.
RadioGroup, Radio has separate top level documentation pages when Radio is meant to be used inside of a RadioGroup. This conflicts with other components like Combobox, Dropdown, TabList, MenuList, etc. which do not have documentation pages for their subcomponents (Option, MenuItem, etc).
Card is an example of a component which uses folders to nest separate documentation for subcomponents, even though those subcomponents are not usable outside of a Card.
The Breadcrumb collection has a similar issue to Card in that it documents subcomponents that are unusable outside the Breadcrumb. However, it also does so using a separate top level story for each sub component vs a folder.
Virtualizer has 3 separate top level components, which are functional on their own, and are documented as 3 separate top level stories. However, this conflicts (in part) with Tree which uses a folder to group similar top level stories.
Tree itself combines several of the above issues as it uses a folder to organize its group of stories but also includes a top level story for
TreeItemwhich is not usable on its own.📸 Anatomy Screenshots
We've included only one screenshot for each component, however, the prototype on this branch applied the highlighting to all stories. We also included any extra stories for a given component if it had issues with the anatomy.
Components
Accordion
When you click the Accordion Header, it opens and the anatomy selectors automatically update. They are currently prototyped on a RAF. Notice
4the AccordionPanel is now highlighted.Avatar
Badge story shown. Notice the selection boxes only grab the first instance in a list.
On hover of the list of anatomy, all instances are highlighted. Here we highlight
bbadge and all Avatar badges get the same highlight as the anatomy list being hovered:AvatarGroup
Notice since the AvatarGroup is a component itself, the
Avataranatomy is not listed, only theAvatarGroupanatomy. This is by design so each story focuses only on the anatomy relevant to the story itself, not every component used inside the story.Badge
Counter Badge
💬 Discuss
CounterBadgecomposesBadgeand the current prototype only shows components which explicitly match the component page being shown. So,Badgeis excluded since this is theCounterBadgepage.PresenceBadge
The final implementation may want to consider drawing lines from the indicator dot to the anatomy box to prevent obscuring the component like this example does.
Button
CompoundButton
MenuButton
SplitButton
ToggleButton
Card
CardFooter
CardHeader
This is a dense story with many examples of a single component implemented in various ways. The usefulness of the highlighting feature is shown in the video.
CardHeader.mov
CardPreview
Checkbox
Combobox
👁️ See Collection Component Docs.
💬 Discuss This raises the topic of collection components (Combobox, Dropdown, RadioGroup, TabList). Our conventions for these vary in terms of structure, naming, and composition.
Combox does not follow Fluent UI React v9 naming patterns for its anatomy. It consists of:
The anatomy view does not capture many configurable slots due to the naming and structure differences of this component.
If the naming were to follow the
ComboBox*pattern for components andComboBox*__*pattern for slots, we might see something like this where each component/slot is a type of "Combobox":DataGrid
This is a dense view that is difficult to read on first glance, however, it is indicative of the component anatomy which is truly complex.
When hovering on various parts the anatomy is much more useful for understanding the structure than clicking through
tsxfiles or looking at the source code.data-grid.mov
The anatomy tool does not highlight anything in the "Virtualization" story, however, upon inspecting this example is just using an iframe. So, the tool in this case is again useful as it is catching an issue in our docs. We likely should not be shipping stories that are iframe'd to other repos:
Dialog
Note, the prototype doesn't provide and good experience for hovering over anatomy pieces when there is a backdrop.
Divider
Dropdown
👁️ See Collection Component Docs.
Field
In this example it is immediately clear that the cat icon and input are custom elements as they have no anatomy annotation. The icon would be an easy element to assume to come from Fluent UI or at least have a slot for input icons. Without this tool, it is unclear what is custom and what is provided by Fluent UI.
FluentProvider
Image
Input
Label
Link
Menu
👁️ See Collection Component Docs.
MenuList
👁️ See Collection Component Docs.
If we captured all
Menu*named components, not justMenuList*, then the anatomy would look like this:Overflow
Overflow is a utility with no DOM, so anatomy annotations do not apply here.
Persona
Popover
Portal
Nothing to see here, utility component only, no DOM.
ProgressBar
Radio
RadioGroup
👁️ See Collection Component Docs.
Select
👁️ See Collection Component Docs.
Skeleton
Slider
SpinButton
Spinner
Switch
Table
👁️ See DataGrid comments which also apply to Table.
TabList
👁️ See Collection Component Docs.
Text
Textarea
Toast
Even though it is useful,
Toasteris included in the anatomy by a bug in the component name matcher. Toaster starts withToast. This should technically be fixed in a final version as component anatomies that start with the currently viewed component name should not be included in the view, only actual subcomponents and slots.Toolbar
👁️ See Collection Component Docs.
The
Toolbarstory usesToolbar,ToolbarButtonandToolbarDividerbut only showsToolbarin the anatomy as the other components do not follow the class name convention for composing components. They are missingfui-Toolbar*classes.Tooltip
Compat Components
These components are v8 made to work with v9 styling and tokens, but do not follow v9 architecture.
DatePicker
All v9 components are properly shown.
Preview Components
Alert
Breadcrumb
👁️ See
BreadcrumbButtoncomment.BreadcrumbButton
💬 Discuss This should probably be combined into the
Breadcrumbstories page.BreadcrumbButtonis not usable on its own. There is one story, which shows the use ofBreadcrumb,BreadcrumbItem, andBreadcrumbButtontogether.As a separate issue, the story should be simplified to not include boilerplate for the "playground" type code around the story. There is no need to show the appearance and size in a single story, but it should be separate concise stories for each capability of the
BreadcrumbButton.Note, this type of finding and review is not a direct result of anatomy work, but it is due to the audit work that is required by the anatomy work. We should have a process for doing this type of audit regularly. At the least, it should be a step required for promoting preview components to proper components.
BreadcrumbDivider
👁️ See
BreadcrumbButtoncomment.BreadcrumbItem
👁️ See
BreadcrumbButtoncomment.BreadcrumbLink
👁️ See
BreadcrumbButtoncomment.Drawer
The prototype does not handle anatomy annotation labels well when a component is full-bleed (to the edge of the iframe area). This could be fixed by a smarter label placement algorithm which we did not do.
InfoButton
InfoLabel
SearchBox
💬 Discuss The
contentBeforeslot is "missing" from the anatomy view. Why?The SearchBox internally uses the
Inputcomponent as its root. ThecontentBeforeslot is passed through the SearchBox state on toInput's props. However, since SearchBox does no special handling of thecontentBefore, there is no SearchBox className added to this slot. NoticecontentAfterwhich does appear in the anatomy, because SearchBox has special handling of this Input prop and adds aSearchBox__contentAfterclassName here.Summary of findings:
This raises a few questions about composition:
fui-SearchBox__contentBeforebecause it is using this slot from Input and does not make any modifications to it.Original PR comment: #28090 (comment)
Tree/flatTree
Tree/Layouts
Tree/Tree
👁️ See Collection Component Docs.
Tree/TreeItem
💬 Discuss
TreeItemhas a duplicate className offui-TreeItemLayout__expandIcon fui-TreeItemLayout__expandIconon the expandIcon slot. We could not find why. This is relatively low consequence but still probably a bug we should fix.Virtualizer
On first glance, the anatomy view is not useful. On reconsideration, it does reveal how the virtualizer works.
You can see how the
before,after, andafterContainerare used to generate the list on scroll. This would be useful for debugging and for developing features such as loading indicators or placeholders:You can also see that the
Virtualizerdoes not render anything to the DOM, as indicated by the grayed out(?) Virtualizerlabel in the list of anatomy.VirtualizerScrollView
💬 Discuss Nothing shows in the view for multiple reasons:
fui-VirtualizerScrollViewand usesfui-Virtualizer-Scroll-View.Virtualizerwe don't currently follow a predictable pattern or convention, so the anatomy viewer fails to show all the composed components/slots.1 - Class Name
Fixing the className to use
fui-VirtualizerScrollView, we get acontainerslot visible. However,VirtualizerScrollViewalso composesVirtualizerbut we don't see any of those components or slots.2 - Composition
Hacking the story and anatomy view to show all, we see this:
VirtualizerScrollViewDynamic
👁️ See Preview Components
VirtualizerScrollViewdiscussion. ThisVirtualizerScrollViewDynamiccomponent has identical patterns and issues.