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

fix(useEventListener): avoid double handlers calls in React 17 #16514

Merged
merged 3 commits into from
Jan 18, 2021

Conversation

layershifter
Copy link
Member

Fixes Semantic-Org/Semantic-UI-React#4110.

On Fluent N* side we will also have the same issues with Portals, see facebook/react#20074 for more details. This PR performs a similar workaround as react-bootstrap/react-overlays#880.

@codesandbox-ci
Copy link

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 1cf043d:

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

@fabricteam
Copy link
Collaborator

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 872 882 5000
BaseButtonCompat mount 951 961 5000
Breadcrumb mount 43579 43739 5000
Checkbox mount 1575 1564 5000
CheckboxBase mount 1345 1382 5000
ChoiceGroup mount 5049 4973 5000
ComboBox mount 1000 1001 1000
CommandBar mount 10453 10424 1000
ContextualMenu mount 6248 6238 1000
DefaultButtonCompat mount 1218 1233 5000
DetailsRow mount 3764 3763 5000
DetailsRowFast mount 3812 3859 5000
DetailsRowNoStyles mount 3597 3602 5000
Dialog mount 1569 1547 1000
DocumentCardTitle mount 1781 1823 1000
Dropdown mount 3457 3512 5000
FocusTrapZone mount 1850 1862 5000
FocusZone mount 1845 1917 5000
IconButtonCompat mount 1946 1887 5000
Label mount 345 351 5000
Layer mount 1886 1843 5000
Link mount 506 491 5000
MakeStyles mount 2056 2018 50000
MenuButtonCompat mount 1548 1588 5000
MessageBar mount 2020 2069 5000
Nav mount 3439 3452 1000
OverflowSet mount 1098 1085 5000
Panel mount 1532 1500 1000
Persona mount 877 895 1000
Pivot mount 1484 1456 1000
PrimaryButtonCompat mount 1334 1380 5000
Rating mount 8082 8039 5000
SearchBox mount 1429 1412 5000
Shimmer mount 2753 2719 5000
Slider mount 2051 1969 5000
SpinButton mount 5184 5292 5000
Spinner mount 436 437 5000
SplitButtonCompat mount 3279 3374 5000
Stack mount 521 558 5000
StackWithIntrinsicChildren mount 1580 1591 5000
StackWithTextChildren mount 4871 4754 5000
SwatchColorPicker mount 10774 10577 5000
Tabs mount 1480 1481 1000
TagPicker mount 2943 2951 5000
TeachingBubble mount 12108 11816 5000
Text mount 454 435 5000
TextField mount 1471 1515 5000
ThemeProvider mount 2233 2213 5000
ThemeProvider virtual-rerender 655 654 5000
Toggle mount 814 876 5000
button mount 716 705 5000
buttonNative mount 118 112 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.19 0.54 0.35:1 2000 377
🦄 Button.Fluent 0.13 0.22 0.59:1 5000 641
🔧 Checkbox.Fluent 0.7 0.36 1.94:1 1000 698
🎯 Dialog.Fluent 0.17 0.23 0.74:1 5000 874
🔧 Dropdown.Fluent 3.11 0.42 7.4:1 1000 3113
🔧 Icon.Fluent 0.15 0.06 2.5:1 5000 756
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 447
🔧 Slider.Fluent 1.61 0.46 3.5:1 1000 1610
🔧 Text.Fluent 0.09 0.03 3:1 5000 425
🦄 Tooltip.Fluent 0.13 0.91 0.14:1 5000 631

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
Checkbox.Fluent 698 637 1.1:1
DividerMinimalPerf.default 452 414 1.09:1
ListCommonPerf.default 775 708 1.09:1
TreeWith60ListItems.default 211 194 1.09:1
HeaderMinimalPerf.default 444 413 1.08:1
CardMinimalPerf.default 663 620 1.07:1
ChatWithPopoverPerf.default 497 464 1.07:1
LabelMinimalPerf.default 520 485 1.07:1
ListMinimalPerf.default 597 559 1.07:1
AlertMinimalPerf.default 347 326 1.06:1
ButtonMinimalPerf.default 219 206 1.06:1
ChatMinimalPerf.default 688 652 1.06:1
HeaderSlotsPerf.default 906 854 1.06:1
RefMinimalPerf.default 266 250 1.06:1
AvatarMinimalPerf.default 248 236 1.05:1
ButtonSlotsPerf.default 643 613 1.05:1
Icon.Fluent 756 717 1.05:1
DropdownManyItemsPerf.default 835 804 1.04:1
TreeMinimalPerf.default 915 880 1.04:1
CarouselMinimalPerf.default 528 515 1.03:1
CheckboxMinimalPerf.default 3051 2954 1.03:1
ItemLayoutMinimalPerf.default 1374 1330 1.03:1
ListNestedPerf.default 652 631 1.03:1
ListWith60ListItems.default 712 688 1.03:1
PortalMinimalPerf.default 178 172 1.03:1
ReactionMinimalPerf.default 466 451 1.03:1
SegmentMinimalPerf.default 411 400 1.03:1
StatusMinimalPerf.default 803 779 1.03:1
IconMinimalPerf.default 739 718 1.03:1
VideoMinimalPerf.default 747 727 1.03:1
Tooltip.Fluent 631 615 1.03:1
ButtonUseCssPerf.default 907 891 1.02:1
ButtonUseCssNestingPerf.default 1156 1133 1.02:1
ImageMinimalPerf.default 438 429 1.02:1
LoaderMinimalPerf.default 773 759 1.02:1
SplitButtonMinimalPerf.default 4121 4058 1.02:1
Avatar.Fluent 377 370 1.02:1
Button.Fluent 641 631 1.02:1
Image.Fluent 447 439 1.02:1
BoxMinimalPerf.default 429 423 1.01:1
DatepickerMinimalPerf.default 50272 49894 1.01:1
DropdownMinimalPerf.default 3081 3051 1.01:1
FormMinimalPerf.default 488 484 1.01:1
MenuMinimalPerf.default 960 947 1.01:1
MenuButtonMinimalPerf.default 1731 1721 1.01:1
RadioGroupMinimalPerf.default 511 506 1.01:1
SliderMinimalPerf.default 1633 1624 1.01:1
TooltipMinimalPerf.default 869 862 1.01:1
Dialog.Fluent 874 862 1.01:1
AnimationMinimalPerf.default 449 449 1:1
ButtonOverridesMissPerf.default 1773 1781 1:1
ChatDuplicateMessagesPerf.default 395 395 1:1
EmbedMinimalPerf.default 4314 4329 1:1
InputMinimalPerf.default 1375 1377 1:1
PopupMinimalPerf.default 745 745 1:1
ProviderMergeThemesPerf.default 1626 1627 1:1
TableMinimalPerf.default 463 465 1:1
CustomToolbarPrototype.default 3837 3825 1:1
ToolbarMinimalPerf.default 1050 1053 1:1
Dropdown.Fluent 3113 3109 1:1
Slider.Fluent 1610 1613 1:1
AttachmentSlotsPerf.default 1293 1312 0.99:1
DialogMinimalPerf.default 851 857 0.99:1
LayoutMinimalPerf.default 462 465 0.99:1
TableManyItemsPerf.default 2336 2358 0.99:1
AttachmentMinimalPerf.default 185 190 0.97:1
GridMinimalPerf.default 399 411 0.97:1
ProviderMinimalPerf.default 980 1012 0.97:1
TextMinimalPerf.default 409 422 0.97:1
TextAreaMinimalPerf.default 567 584 0.97:1
AccordionMinimalPerf.default 179 187 0.96:1
FlexMinimalPerf.default 329 341 0.96:1
RosterPerf.default 1279 1337 0.96:1
SkeletonMinimalPerf.default 411 431 0.95:1
Text.Fluent 425 446 0.95:1

@size-auditor
Copy link

size-auditor bot commented Jan 18, 2021

Asset size changes

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

Baseline commit: 134d782c8cedf6b2da632728d13ed433f0063279 (build)

@layershifter layershifter merged commit 20f3d66 into master Jan 18, 2021
@layershifter layershifter deleted the fix/use-listnener-r17 branch January 18, 2021 14:11
ling1726 pushed a commit to ling1726/fluentui that referenced this pull request Jan 18, 2021
…soft#16514)

* fix(useEventListener): avoid double handlers calls in React 17

* add changelog entry
ling1726 added a commit to ling1726/fluentui that referenced this pull request Jan 18, 2021
ling1726 added a commit that referenced this pull request Jan 18, 2021
…n React 17 #16514 (#16516)

* fix(useEventListener): avoid double handlers calls in React 17 (#16514)

* fix(useEventListener): avoid double handlers calls in React 17

* add changelog entry

* chore(Backport): fix(useEventListener): avoid double handlers calls in React 17 #16514

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>
YuanboXue-Amber pushed a commit that referenced this pull request Jan 19, 2021
…n React 17 #16514 (#16516)

* fix(useEventListener): avoid double handlers calls in React 17 (#16514)

* fix(useEventListener): avoid double handlers calls in React 17

* add changelog entry

* chore(Backport): fix(useEventListener): avoid double handlers calls in React 17 #16514

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>
assuncaocharles pushed a commit to assuncaocharles/fluentui that referenced this pull request Feb 22, 2021
…soft#16514)

* fix(useEventListener): avoid double handlers calls in React 17

* add changelog entry
assuncaocharles added a commit that referenced this pull request Feb 22, 2021
* fix(useEventListener): use a proper .event and clean up properly (#16991)

* fix(useEventListener): use a proper .event and clean up properly

* update impl to use refs

* revert refs, use settimeout reset

* add changelog entry

* Export tree context (#16891)

* export tree context

* update changelog

* fix(Dropdown): Add missing prop type (#16920)

* Add missing prop type

* Add changelog

* fix(Tree): change selectionIndicator from visibility:hidden to display:none (#16922)

…y:none

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

As described in title.
`visibility: hidden` cause problem because the selectionIndicator still takes width

(optional)

* fix(useEventListener): avoid double handlers calls in React 17 (#16514)

* fix(useEventListener): avoid double handlers calls in React 17

* add changelog entry

* fix(MergeVariablesOverrides): return variables if they are defined (#16560)

* fix(MergeVariablesOverrides): return variables if they are defined

* fix(MergeVariablesOverrides): add changelog

* Update packages/fluentui/react-bindings/src/utils/mergeVariablesOverrides.ts

Co-authored-by: ling1726 <lingfangao@hotmail.com>

* fix(MergeVariablesOverrides): check for plain objects

* fix(MergeVariablesOverrides): fix test

Co-authored-by: ling1726 <lingfangao@hotmail.com>

* fix(react-context-selector): fix React warnings about setState() (#16714)

* WIP on fixing contextSelector

* fix(ContextSelector): fix

* fix(ContextSelector): selectors

* fix(ContextSelector): selectors tests fix

* fix(ContextSelector): changelog

* fix(ContextSelector): fix rollup

* fix(ContextSelector): flatten context value

* fix(ContextSelector): add comments

* Update packages/fluentui/CHANGELOG.md

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* fix(ContextSelector): move scheduler to peerDependencies

* Update packages/fluentui/react-context-selector/src/types.ts

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Update packages/fluentui/react-context-selector/src/types.ts

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Update packages/fluentui/react-context-selector/src/types.ts

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Update packages/fluentui/react-context-selector/src/createContext.ts

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Update packages/fluentui/react-context-selector/src/useContextSelector.ts

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Update packages/fluentui/react-context-selector/package.json

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>
Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* feat(Button): disabledFocusable for v0 Button (#16419)

* button disabled focusable for v0

* fixes

* changelog

* disable  pointer events only for disabled, not for disabledFocusable

* fix

* cleanup examples

* small revert

Co-authored-by: Juraj Kapsiar <jukapsia@microsoft.com>

* adding Channel Share Icon for ODSP (#16619)

* adding Channel Share Icon

* feat(FormTextArea): Add new FormTextArea component (#16660)

* feat(FormTextArea): add new FormTextArea component

* feat(FormTextArea): add test

* feat(FormTextArea): change type

* feat(FormTextArea): add changelog

* feat(TextArea): fix changelog

* Update packages/fluentui/react-northstar/src/components/TextArea/TextArea.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* feat(TextArea): add error exampel

* Update packages/fluentui/docs/src/examples/components/Form/Usage/FormExampleErrorAndSuccessful.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* feat(checkbox): add indeterminate support (#16081)

* feat(checkbox): add indeterminate support

* feat(checkbox): add props comment

* feat(checkbox): add best practices

* feat(checkbox): add changelog

* feat(checkbox): rename indicator

* feat(Checkbox): add mixed value support

* feat(Checkbox): remove inderminate prop

* feat(Checkbox): remove inderminate from state

* feat(Checkbox): remove cast

* Update packages/fluentui/CHANGELOG.md

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* feat(Checkbox): remove indeterminate from behavior

* feat(Checkbox): remove unecessary check

* feat(Checkbox): rename best practices

* feat(Checkbox): remove indeterminate from styles

* feat(Checkbox): rename example

* feat(Checkbox): update contorl

* Update packages/fluentui/react-northstar/src/themes/teams/components/Checkbox/checkboxStyles.ts

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Update packages/fluentui/react-northstar/src/themes/teams/components/Checkbox/checkboxStyles.ts

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Update packages/fluentui/docs/src/examples/components/Checkbox/States/index.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* feat(Checkbox): update styles

* feat(Checkbox): screener

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Adding imageAltText icon (#16884)

* adding imageAltText icon

* fix(felaRenderer): exclude className for CKEditor to avoid collisions (#17025)

* fix(felaRenderer): exclude className for CKEditor to avoid collisions

* add changelog entry

* Update packages/fluentui/react-northstar-fela-renderer/src/createFelaRenderer.tsx

* Update packages/fluentui/react-northstar-fela-renderer/src/createFelaRenderer.tsx

* fix(useEventListener): use a proper .event and clean up properly (#16991)

* fix(useEventListener): use a proper .event and clean up properly

* update impl to use refs

* revert refs, use settimeout reset

* add changelog entry

* fix(popupBehavior): remove role='complementary' from inline popup (#17032)

* fix(popupBehavior): remove role='complementary' from inline popup

* changelog

* add popup test

* chore: Automated release pipeline for fluent N* (#16866)

* add pack-nightly task

* Add nightly package to docsite

* Add post publish validation script

* Add canary release script

* Fix dep url when pack nightly tarball

* yaml pipeline

* update redirect

* fix pipeline

* rename

* nightly build fix

* remove todo

* gzip true

* make dropdown display nightly version properly

* update comment

* update pipeline

* put 0.0.0-nightly into const

* Update scripts/tasks/fluentui-publish.ts

Co-authored-by: ling1726 <lingfan.gao@microsoft.com>

* Update azure-pipelines.release-fluentui.yml

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* update pipeline

* update pipeline

* update pipeline

* hide clip board for nightly release

* Update azure-pipelines.release-fluentui.yml

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update scripts/tasks/fluentui-publish.ts

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* Update scripts/tasks/fluentui-publish.ts

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

* update packtarball

* test pipeline

* Update falsy check of nightlyRelease

* fix doc build

* fix

* remove cruft

* separate fluent ui tasks

* store nightly tarballs in Build.ArtifactStagingDirectory

* cleanup after tar; add --no-verify-access

* add no-verify-access comment

* update after trying out real release

* add pipeline name

* update pipeline description

* Update scripts/fluentui-publish/index.ts

Co-authored-by: ling1726 <lingfan.gao@microsoft.com>

* fix screener build

* test adding nightly date to nightly tarball versions

* Update scripts/fluentui-publish/index.ts

Co-authored-by: ling1726 <lingfan.gao@microsoft.com>

* use newest folder for docsite

* Revert "use newest folder for docsite"

This reverts commit 2b8af70.

* make codesandbox consistent for nightly release

Co-authored-by: ling1726 <lingfan.gao@microsoft.com>
Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>

Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>
Co-authored-by: Amber <yuanboxue@microsoft.com>
Co-authored-by: Roman Sudarikov <pompomon@users.noreply.github.com>
Co-authored-by: ling1726 <lingfangao@hotmail.com>
Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
Co-authored-by: Juraj Kapsiar <jurokapsiar@gmail.com>
Co-authored-by: Juraj Kapsiar <jukapsia@microsoft.com>
Co-authored-by: Andrew Martin <andmarti@microsoft.com>
Co-authored-by: ling1726 <lingfan.gao@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modal kills the Pushable Sidebar
4 participants