-
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
[Northstar][Dropdown] Fix styling mutation when merging themes #24787
Conversation
📊 Bundle size report🤖 This report was generated against e23edb70bfcde6fc5f82b676a1ddcf8d5a854dbd |
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 97c53a8:
|
Perf Analysis (
|
Scenario | Current PR Ticks | Baseline Ticks | Ratio |
---|---|---|---|
ChatWithPopoverPerf.default | 307 | 283 | 1.08:1 |
ListNestedPerf.default | 480 | 451 | 1.06:1 |
ListWith60ListItems.default | 501 | 480 | 1.04:1 |
LoaderMinimalPerf.default | 528 | 510 | 1.04:1 |
RefMinimalPerf.default | 190 | 183 | 1.04:1 |
AttachmentSlotsPerf.default | 872 | 844 | 1.03:1 |
DividerMinimalPerf.default | 318 | 308 | 1.03:1 |
GridMinimalPerf.default | 297 | 289 | 1.03:1 |
HeaderMinimalPerf.default | 322 | 312 | 1.03:1 |
HeaderSlotsPerf.default | 692 | 674 | 1.03:1 |
SkeletonMinimalPerf.default | 308 | 299 | 1.03:1 |
TextAreaMinimalPerf.default | 417 | 405 | 1.03:1 |
AnimationMinimalPerf.default | 473 | 466 | 1.02:1 |
BoxMinimalPerf.default | 296 | 289 | 1.02:1 |
ButtonOverridesMissPerf.default | 1033 | 1010 | 1.02:1 |
DropdownManyItemsPerf.default | 541 | 533 | 1.02:1 |
LabelMinimalPerf.default | 341 | 335 | 1.02:1 |
MenuButtonMinimalPerf.default | 1370 | 1349 | 1.02:1 |
RadioGroupMinimalPerf.default | 396 | 387 | 1.02:1 |
TextMinimalPerf.default | 308 | 301 | 1.02:1 |
TreeWith60ListItems.default | 131 | 129 | 1.02:1 |
AlertMinimalPerf.default | 222 | 220 | 1.01:1 |
CheckboxMinimalPerf.default | 1543 | 1526 | 1.01:1 |
DialogMinimalPerf.default | 693 | 689 | 1.01:1 |
DropdownMinimalPerf.default | 2176 | 2148 | 1.01:1 |
ImageMinimalPerf.default | 341 | 336 | 1.01:1 |
InputMinimalPerf.default | 851 | 843 | 1.01:1 |
ListCommonPerf.default | 509 | 504 | 1.01:1 |
TableMinimalPerf.default | 359 | 355 | 1.01:1 |
ToolbarMinimalPerf.default | 796 | 791 | 1.01:1 |
TreeMinimalPerf.default | 705 | 701 | 1.01:1 |
AccordionMinimalPerf.default | 126 | 126 | 1:1 |
CarouselMinimalPerf.default | 355 | 356 | 1:1 |
ChatMinimalPerf.default | 641 | 639 | 1:1 |
DatepickerMinimalPerf.default | 4679 | 4668 | 1:1 |
FormMinimalPerf.default | 325 | 324 | 1:1 |
ItemLayoutMinimalPerf.default | 967 | 964 | 1:1 |
RosterPerf.default | 1714 | 1715 | 1:1 |
PopupMinimalPerf.default | 556 | 554 | 1:1 |
ProviderMergeThemesPerf.default | 997 | 993 | 1:1 |
ReactionMinimalPerf.default | 335 | 335 | 1:1 |
SliderMinimalPerf.default | 1226 | 1225 | 1:1 |
StatusMinimalPerf.default | 607 | 610 | 1:1 |
TableManyItemsPerf.default | 1569 | 1563 | 1:1 |
TooltipMinimalPerf.default | 1863 | 1854 | 1:1 |
AvatarMinimalPerf.default | 160 | 162 | 0.99:1 |
ButtonMinimalPerf.default | 133 | 135 | 0.99:1 |
CardMinimalPerf.default | 460 | 467 | 0.99:1 |
ChatDuplicateMessagesPerf.default | 228 | 230 | 0.99:1 |
EmbedMinimalPerf.default | 2631 | 2650 | 0.99:1 |
ListMinimalPerf.default | 442 | 446 | 0.99:1 |
MenuMinimalPerf.default | 734 | 739 | 0.99:1 |
PortalMinimalPerf.default | 133 | 134 | 0.99:1 |
SplitButtonMinimalPerf.default | 3256 | 3284 | 0.99:1 |
CustomToolbarPrototype.default | 2148 | 2161 | 0.99:1 |
FlexMinimalPerf.default | 251 | 255 | 0.98:1 |
LayoutMinimalPerf.default | 313 | 318 | 0.98:1 |
ProviderMinimalPerf.default | 314 | 321 | 0.98:1 |
VideoMinimalPerf.default | 607 | 618 | 0.98:1 |
AttachmentMinimalPerf.default | 125 | 129 | 0.97:1 |
ButtonSlotsPerf.default | 422 | 433 | 0.97:1 |
SegmentMinimalPerf.default | 296 | 305 | 0.97:1 |
IconMinimalPerf.default | 550 | 585 | 0.94:1 |
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: e23edb70bfcde6fc5f82b676a1ddcf8d5a854dbd (build) |
Can you please provide a reproduction case for this behavior? |
Here is the codesandbox: https://codesandbox.io/s/fluent-ui-example-forked-c58ijl?file=/example.js
Actual behavior: Expected behavior: |
I have accidentally created the example with older version of Fluent, but the latest (0.64.0) is also affected. |
@petrjaros thanks for providing the repro. The root reason is there: fluentui/packages/fluentui/styles/src/mergeThemes.ts Lines 97 to 100 in a33448f
https://lodash.com/docs/4.17.15#merge I talked with a team and we agreed to merge this PR instead of fixing that call to avoid potential memory issues. Especially taking into account that this call was there for ages. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Pull request was closed
* master: (28 commits) fix: use trigger prop for aria-haspopup (microsoft#24794) chore(react-dialog): scaffold DialogContent component (microsoft#24844) chore: Northstar screener should read from screenerStates.json (microsoft#24848) applying package updates (web components) Standardize focus treatment (microsoft#24771) Divider - allow default prop override (microsoft#24840) GroupedList: fix virtualization (unstable preview) (microsoft#24460) fix: Add explicit children prop to TeachingBubble to support React 18 (microsoft#24823) feat: Adds `visible` prop to `TableCellActions` (microsoft#24831) [Northstar][Dropdown] Fix styling mutation when merging themes (microsoft#24787) fix: export `tableCellActionsClassNames` from unstable (microsoft#24830) bugfix(react-dialog): Adds color style to DialogSurface (microsoft#24832) applying package updates Prevent group toggling from selecting the whole group (microsoft#24822) feat(react-textarea): Add shadow variant of filled appearance (microsoft#24512) applying package updates Adding lib-commonjs top-level entries to exports map (microsoft#24792) Created shim packages (microsoft#24780) feat(react-menu): replace keydown handlers by useARIAButtonShorthand on MenuItem (microsoft#24738) fix: update version mismatches triggered by v9 release (microsoft#24812) ...
Current Behavior
When overriding :active or :hover state it mutates the style object. Style override of single Dropdown stay permanent, and the style is then applied to all unrelated Dropdowns.
New Behavior
Styling of one Dropdown does not leak into other Dropdowns.