-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Updated tooltip stories to support codesandbox #20373
Conversation
📊 Bundle size reportUnchanged fixtures
|
Asset size changesUnable to find bundle size details for Baseline commit: 3d5ca9b Possible causes
Recommendations
|
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 5aa15b6:
|
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 1045 | 1025 | 5000 | |
BaseButton | mount | 1054 | 1080 | 5000 | |
Breadcrumb | mount | 2817 | 2839 | 1000 | |
ButtonNext | mount | 551 | 584 | 5000 | |
Checkbox | mount | 1731 | 1757 | 5000 | |
CheckboxBase | mount | 1538 | 1477 | 5000 | |
ChoiceGroup | mount | 5357 | 5379 | 5000 | |
ComboBox | mount | 1068 | 1064 | 1000 | |
CommandBar | mount | 11149 | 11053 | 1000 | |
ContextualMenu | mount | 7183 | 7207 | 1000 | |
DefaultButton | mount | 1316 | 1282 | 5000 | |
DetailsRow | mount | 4210 | 4314 | 5000 | |
DetailsRowFast | mount | 4179 | 4219 | 5000 | |
DetailsRowNoStyles | mount | 4065 | 4063 | 5000 | |
Dialog | mount | 2705 | 2809 | 1000 | |
DocumentCardTitle | mount | 177 | 180 | 1000 | |
Dropdown | mount | 3650 | 3603 | 5000 | |
FluentProviderNext | mount | 3535 | 3461 | 5000 | |
FluentProviderWithTheme | mount | 218 | 226 | 10 | |
FluentProviderWithTheme | virtual-rerender | 104 | 117 | 10 | |
FluentProviderWithTheme | virtual-rerender-with-unmount | 262 | 253 | 10 | |
FocusTrapZone | mount | 1973 | 1907 | 5000 | |
FocusZone | mount | 1911 | 1931 | 5000 | |
IconButton | mount | 1970 | 1986 | 5000 | |
Label | mount | 383 | 393 | 5000 | |
Layer | mount | 3352 | 3347 | 5000 | |
Link | mount | 543 | 551 | 5000 | |
MakeStyles | mount | 1926 | 1954 | 50000 | |
MenuButton | mount | 1703 | 1712 | 5000 | |
MessageBar | mount | 2165 | 2151 | 5000 | |
Nav | mount | 3735 | 3664 | 1000 | |
OverflowSet | mount | 1226 | 1242 | 5000 | |
Panel | mount | 2708 | 2708 | 1000 | |
Persona | mount | 946 | 948 | 1000 | |
Pivot | mount | 1613 | 1607 | 1000 | |
PrimaryButton | mount | 1488 | 1439 | 5000 | |
Rating | mount | 8941 | 8823 | 5000 | |
SearchBox | mount | 1558 | 1569 | 5000 | |
Shimmer | mount | 2851 | 2969 | 5000 | |
Slider | mount | 2180 | 2190 | 5000 | |
SpinButton | mount | 5635 | 5586 | 5000 | |
Spinner | mount | 466 | 479 | 5000 | |
SplitButton | mount | 3571 | 3573 | 5000 | |
Stack | mount | 589 | 612 | 5000 | |
StackWithIntrinsicChildren | mount | 2033 | 1937 | 5000 | |
StackWithTextChildren | mount | 5507 | 5521 | 5000 | |
SwatchColorPicker | mount | 11638 | 11679 | 5000 | |
Tabs | mount | 1593 | 1623 | 1000 | |
TagPicker | mount | 2996 | 2985 | 5000 | |
TeachingBubble | mount | 13997 | 14061 | 5000 | |
Text | mount | 497 | 503 | 5000 | |
TextField | mount | 1565 | 1613 | 5000 | |
ThemeProvider | mount | 1283 | 1307 | 5000 | |
ThemeProvider | virtual-rerender | 662 | 666 | 5000 | |
ThemeProvider | virtual-rerender-with-unmount | 2148 | 2111 | 5000 | |
Toggle | mount | 949 | 952 | 5000 | |
buttonNative | mount | 151 | 141 | 5000 |
Perf Analysis (@fluentui/react-northstar
)
Perf tests with no regressions
Scenario | Current PR Ticks | Baseline Ticks | Ratio |
---|---|---|---|
TreeWith60ListItems.default | 232 | 189 | 1.23:1 |
TextAreaMinimalPerf.default | 634 | 566 | 1.12:1 |
ButtonMinimalPerf.default | 222 | 200 | 1.11:1 |
ChatDuplicateMessagesPerf.default | 365 | 336 | 1.09:1 |
GridMinimalPerf.default | 405 | 373 | 1.09:1 |
PortalMinimalPerf.default | 197 | 184 | 1.07:1 |
RadioGroupMinimalPerf.default | 527 | 491 | 1.07:1 |
VideoMinimalPerf.default | 744 | 694 | 1.07:1 |
ListNestedPerf.default | 666 | 626 | 1.06:1 |
CardMinimalPerf.default | 676 | 644 | 1.05:1 |
DividerMinimalPerf.default | 420 | 401 | 1.05:1 |
HeaderSlotsPerf.default | 885 | 846 | 1.05:1 |
TooltipMinimalPerf.default | 1235 | 1177 | 1.05:1 |
AttachmentSlotsPerf.default | 1215 | 1165 | 1.04:1 |
SegmentMinimalPerf.default | 413 | 396 | 1.04:1 |
StatusMinimalPerf.default | 785 | 756 | 1.04:1 |
TreeMinimalPerf.default | 915 | 882 | 1.04:1 |
AttachmentMinimalPerf.default | 186 | 180 | 1.03:1 |
AvatarMinimalPerf.default | 232 | 225 | 1.03:1 |
CarouselMinimalPerf.default | 538 | 521 | 1.03:1 |
DropdownManyItemsPerf.default | 796 | 771 | 1.03:1 |
EmbedMinimalPerf.default | 4782 | 4640 | 1.03:1 |
HeaderMinimalPerf.default | 425 | 411 | 1.03:1 |
ImageMinimalPerf.default | 433 | 420 | 1.03:1 |
ListCommonPerf.default | 750 | 731 | 1.03:1 |
MenuMinimalPerf.default | 954 | 929 | 1.03:1 |
SkeletonMinimalPerf.default | 418 | 406 | 1.03:1 |
IconMinimalPerf.default | 706 | 684 | 1.03:1 |
TableMinimalPerf.default | 464 | 450 | 1.03:1 |
ButtonSlotsPerf.default | 638 | 623 | 1.02:1 |
FormMinimalPerf.default | 473 | 464 | 1.02:1 |
LabelMinimalPerf.default | 448 | 439 | 1.02:1 |
ChatMinimalPerf.default | 744 | 735 | 1.01:1 |
CheckboxMinimalPerf.default | 3001 | 2965 | 1.01:1 |
InputMinimalPerf.default | 1440 | 1421 | 1.01:1 |
ListMinimalPerf.default | 576 | 568 | 1.01:1 |
MenuButtonMinimalPerf.default | 1832 | 1822 | 1.01:1 |
PopupMinimalPerf.default | 658 | 654 | 1.01:1 |
ReactionMinimalPerf.default | 436 | 431 | 1.01:1 |
TableManyItemsPerf.default | 2194 | 2173 | 1.01:1 |
ToolbarMinimalPerf.default | 1087 | 1074 | 1.01:1 |
DatepickerMinimalPerf.default | 6102 | 6093 | 1:1 |
DialogMinimalPerf.default | 851 | 847 | 1:1 |
LoaderMinimalPerf.default | 762 | 762 | 1:1 |
RefMinimalPerf.default | 269 | 270 | 1:1 |
SliderMinimalPerf.default | 1892 | 1890 | 1:1 |
CustomToolbarPrototype.default | 4514 | 4520 | 1:1 |
DropdownMinimalPerf.default | 3401 | 3431 | 0.99:1 |
FlexMinimalPerf.default | 329 | 333 | 0.99:1 |
ListWith60ListItems.default | 739 | 744 | 0.99:1 |
ProviderMergeThemesPerf.default | 1819 | 1839 | 0.99:1 |
SplitButtonMinimalPerf.default | 4823 | 4856 | 0.99:1 |
TextMinimalPerf.default | 405 | 409 | 0.99:1 |
BoxMinimalPerf.default | 406 | 416 | 0.98:1 |
ItemLayoutMinimalPerf.default | 1343 | 1376 | 0.98:1 |
RosterPerf.default | 1315 | 1346 | 0.98:1 |
ProviderMinimalPerf.default | 1230 | 1251 | 0.98:1 |
ChatWithPopoverPerf.default | 434 | 452 | 0.96:1 |
AccordionMinimalPerf.default | 165 | 174 | 0.95:1 |
AnimationMinimalPerf.default | 458 | 480 | 0.95:1 |
ButtonOverridesMissPerf.default | 1953 | 2060 | 0.95:1 |
LayoutMinimalPerf.default | 398 | 424 | 0.94:1 |
AlertMinimalPerf.default | 314 | 340 | 0.92:1 |
<div>Each of these buttons places the tooltip in a different location relative to its trigger button.</div> | ||
<div className={styles.targetContainer}> | ||
<Tooltip content="above start" positioning="above-start"> | ||
<button style={{ gridArea: '1 / 2' }}>above start</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be using Fluent buttons in these examples. You can check out Menu
and Popover
stories to see it's going to be a bit painful to have those @ts-ignores
but it's the only way to depend on other components until we implement #19866
<Tooltip content="Default tooltip"> | ||
<button>Default</button> | ||
</Tooltip> | ||
<Tooltip content="Inverted tooltip" appearance="inverted"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we take the opportunity in this PR to also break the stories into more atomic stories ? The Default
story is generally a the most basic form of the component that can be modified with the control table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the Tooltip stories are in need of cleanup. It might be better for me to do that in a separate PR, so as not to block this refactoring PR.
@@ -0,0 +1,33 @@ | |||
import * as React from 'react'; | |||
import { Tooltip } from '../Tooltip'; // codesandbox-dependency: @fluentui/react-tooltip ^9.0.0-beta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen these codesandbox-dependency comments before... do we have any tools to verify that they're correct and/or update them if the version changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not currently. They are a workaround pending resolution of #19866
<Tooltip content="Default tooltip"> | ||
<button>Default</button> | ||
</Tooltip> | ||
<Tooltip content="Inverted tooltip" appearance="inverted"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the Tooltip stories are in need of cleanup. It might be better for me to do that in a separate PR, so as not to block this refactoring PR.
Co-authored-by: Oleksandr Fediashov <[email protected]>
@behowell I'll let you take a second pass on these stories to factor them and use other components like Button. This PR can focus only on fixing the codesandbox references. |
* Updated stories to support codesandbox * Change files * Update packages/react-tooltip/src/stories/TooltipAria.stories.tsx Co-authored-by: Oleksandr Fediashov <[email protected]> Co-authored-by: Oleksandr Fediashov <[email protected]>
Pull request checklist
$ yarn change
Description of changes