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 null ref in use slider #24728

Merged
merged 2 commits into from
Sep 8, 2022
Merged

Fix null ref in use slider #24728

merged 2 commits into from
Sep 8, 2022

Conversation

MLoughry
Copy link
Contributor

@MLoughry MLoughry commented Sep 8, 2022

Current Behavior

When you click on a , and the component unmounts while the mouse button is down, and you then move the mosue while the mouse button is still down, there is a null-ref in useSlider()

New Behavior

There is no null-ref errror, as the window mouseMove events are cleaned up on unmount

Related Issue(s)

Fixes #24727

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 8, 2022

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 642bb52:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Sep 8, 2022

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Slider 53.987 kB 54.053 kB ExceedsBaseline     66 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: d653291a07d843930da4023e599177686a1e6e1d (build)

@fabricteam
Copy link
Collaborator

📊 Bundle size report

🤖 This report was generated against d653291a07d843930da4023e599177686a1e6e1d

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
buttonNative mount 91 109 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 767 762 5000
Breadcrumb mount 2343 2341 1000
Checkbox mount 2238 2220 5000
CheckboxBase mount 1946 1938 5000
ChoiceGroup mount 3822 3808 5000
ComboBox mount 758 741 1000
CommandBar mount 9221 8798 1000
ContextualMenu mount 9692 9612 1000
DefaultButton mount 921 947 5000
DetailsRow mount 3027 3015 5000
DetailsRowFast mount 2963 3029 5000
DetailsRowNoStyles mount 2883 2894 5000
Dialog mount 2549 2521 1000
DocumentCardTitle mount 139 140 1000
Dropdown mount 2765 2756 5000
FocusTrapZone mount 1531 1541 5000
FocusZone mount 1538 1512 5000
IconButton mount 1367 1373 5000
Label mount 334 323 5000
Layer mount 3670 3676 5000
Link mount 417 431 5000
MenuButton mount 1185 1184 5000
MessageBar mount 1944 1880 5000
Nav mount 2666 2679 1000
OverflowSet mount 995 995 5000
Panel mount 2056 2077 1000
Persona mount 807 820 1000
Pivot mount 1115 1102 1000
PrimaryButton mount 1074 1071 5000
Rating mount 6540 6498 5000
SearchBox mount 1078 1061 5000
Shimmer mount 2452 2479 5000
Slider mount 1663 1675 5000
SpinButton mount 3845 3806 5000
Spinner mount 394 401 5000
SplitButton mount 2450 2391 5000
Stack mount 456 467 5000
StackWithIntrinsicChildren mount 1839 1829 5000
StackWithTextChildren mount 4547 4509 5000
SwatchColorPicker mount 9073 9029 5000
TagPicker mount 1938 1932 5000
TeachingBubble mount 73162 73000 5000
Text mount 394 388 5000
TextField mount 1134 1150 5000
ThemeProvider mount 1015 1020 5000
ThemeProvider virtual-rerender 710 722 5000
ThemeProvider virtual-rerender-with-unmount 1580 1586 5000
Toggle mount 696 717 5000
buttonNative mount 91 109 5000 Possible regression

@MLoughry MLoughry merged commit a07747c into master Sep 8, 2022
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Sep 9, 2022
* master: (63 commits)
  feat: add helper types to assist DOM element handling (microsoft#24722)
  applying package updates
  Textarea/hc bug (microsoft#24701)
  Fix null ref in use slider (microsoft#24728)
  Add Field unit tests, and remove snapshot tests (microsoft#24706)
  Stress Test: add build commands (microsoft#24575)
  Coachmark - useOffsetHeight should cause re-render on each set state to match v7 functionality (microsoft#24702)
  Implement screener tests for Field components (microsoft#24684)
  Update Field types to clean up react-field.api.md (microsoft#24703)
  fix(Popup): remove rotate(360deg) from PopupContent content styles (microsoft#24432)
  fix(FocusZone): should reset tabindex when focus is outside the zone with prop `shouldResetActiveElementWhenTabFromZone` (microsoft#24463)
  Fix greyed out legend key contrast ratio (microsoft#24714)
  fix: Portal compat should apply `focus-visible` ponyfill (microsoft#24712)
  Fix artifact error (microsoft#24717)
  chore(react-dialog): remove localShorthands in favor of griffel shorthands (microsoft#24715)
  Skip screener checks for draft PRs with exception of appropriately la… (microsoft#24694)
  fix: Remove provider classname from focus styles (microsoft#24710)
  feat: autocontrolled `useTable` hook (microsoft#24688)
  feat: add dialog properties to getNativeElementProps (microsoft#24698)
  Using migrate rather than upgrade term (microsoft#24695)
  ...
@khmakoto khmakoto deleted the fix-null-ref-in-useSlider branch April 5, 2023 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Null reference in useSlider() when <Slider /> unmounts before window mouseUp event fires
6 participants