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.

Sign up for GitHub

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

Jump to bottom

fix: clear Modal timer on component unmount #32969

Merged
merged 1 commit into from
Oct 3, 2024
Merged

fix: clear Modal timer on component unmount #32969

merged 1 commit into from
Oct 3, 2024

Conversation

spmonahan
Copy link
Contributor

Previous Behavior

Modal animation timer was not cleared on unmount.

New Behavior

Modal animation timer is cleared on unmount.

Related Issue(s)

We don't have a repro case so it's not possible to be certain this fixes the issue but it is potentially a problem regardless.

@spmonahan
fix: clear Modal timer on component unmount
fa219f6 
The timer in question is potentially somewhat long-lived so this
ensures we don't invoke the callback after unmount.
@spmonahan spmonahan requested a review from a team as a code owner October 2, 2024 16:43
@github-actions github-actions bot added this to the October Project Cycle Q4 2024 milestone Oct 2, 2024
@fabricteam
Copy link
Collaborator

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react
Dialog		 211.541 kB
63.192 kB		 211.612 kB
63.202 kB		 71 B
10 B
react
Modal		 93.676 kB
30.267 kB		 93.747 kB
30.278 kB		 71 B
11 B
react
Fluent UI React (entire library)		 1.014 MB
282.115 kB		 1.014 MB
282.131 kB		 71 B
16 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react
ActivityItem		 71.236 kB
23.357 kB
react
Announced		 38.493 kB
13.297 kB
react
Autofill		 15.428 kB
4.768 kB
react
Breadcrumb		 202.234 kB
60.43 kB
react
Button		 195.686 kB
56.66 kB
react
ButtonGrid		 180.62 kB
54.643 kB
react
Calendar		 121.776 kB
37.029 kB
react
Callout		 84.33 kB
27.648 kB
react
Check		 53.204 kB
17.848 kB
react
Checkbox		 59.985 kB
19.903 kB
react
ChoiceGroup		 65.494 kB
21.489 kB
react
ChoiceGroupOption		 58.767 kB
19.362 kB
react
Coachmark		 93.151 kB
29.428 kB
react
Color		 7.789 kB
3.127 kB
react
ColorPicker		 135.241 kB
42.223 kB
react
ComboBox		 252.123 kB
72.332 kB
react
CommandBar		 203.275 kB
60.174 kB
react
ContextualMenu		 155.133 kB
48.198 kB
react
DatePicker		 184.204 kB
56.175 kB
react
DateTimeUtilities		 5.244 kB
1.849 kB
react
DetailsList		 229.978 kB
65.921 kB
react
Divider		 19.603 kB
6.845 kB
react
DocumentCard		 217.227 kB
64.472 kB
react
DragDrop		 8.343 kB
2.724 kB
react
DraggableZone		 34.305 kB
11.509 kB
react
Dropdown		 234.173 kB
68.691 kB
react
ExtendedPicker		 96.795 kB
27.893 kB
react
Fabric		 41.745 kB
14.366 kB
react
Facepile		 210.826 kB
63.211 kB
react
FloatingPicker		 242.268 kB
69.042 kB
react
FocusTrapZone		 17.03 kB
5.924 kB
react
FocusZone		 55.053 kB
17.47 kB
react
Grid		 180.62 kB
54.643 kB
react
GroupedList		 135.049 kB
40.758 kB
react
GroupedListV2		 122.638 kB
37.857 kB
react
HoverCard		 97.179 kB
30.82 kB
react
Icon		 51.885 kB
17.272 kB
react
Icons		 66.361 kB
24.397 kB
react
Image		 46.904 kB
15.707 kB
react
Keytip		 81.693 kB
26.766 kB
react
KeytipData		 14.028 kB
4.587 kB
react
KeytipLayer		 103.478 kB
32.017 kB
react
Keytips		 106.239 kB
33.023 kB
react
Label		 38.347 kB
13.257 kB
react
Layer		 48.099 kB
16.367 kB
react
Link		 39.682 kB
13.67 kB
react
List		 39.4 kB
12.481 kB
react
MarqueeSelection		 74.541 kB
22.44 kB
react
MessageBar		 190.75 kB
57.107 kB
react
Nav		 188.226 kB
56.538 kB
react
OverflowSet		 33.393 kB
11.329 kB
react
Overlay		 40.902 kB
14.095 kB
react
Panel		 201.284 kB
60.037 kB
react
Persona		 114.849 kB
36.512 kB
react
PersonaCoin		 114.849 kB
36.512 kB
react
PersonaPresence		 58.074 kB
19.384 kB
react
Pickers		 294.29 kB
82.458 kB
react
Pivot		 189.118 kB
57.275 kB
react
Popup		 12.294 kB
4.195 kB
react
Positioning		 22.807 kB
7.701 kB
react
PositioningContainer		 73.85 kB
23.765 kB
react
ProgressIndicator		 39.504 kB
13.554 kB
react
Rating		 82.014 kB
26.112 kB
react
ResizeGroup		 13.338 kB
4.377 kB
react
ResponsiveMode		 8.13 kB
2.966 kB
react
ScrollablePane		 55.563 kB
17.734 kB
react
SearchBox		 188.999 kB
56.692 kB
react
SelectableOption		 724 B
413 B
react
SelectedItemsList		 232.778 kB
67.938 kB
react
Selection		 42.464 kB
12.301 kB
react
Separator		 35.384 kB
12.146 kB
react
Shimmer		 49.259 kB
16.268 kB
react
ShimmeredDetailsList		 240.76 kB
68.667 kB
react
Slider		 57.651 kB
19.217 kB
react
SpinButton		 192.702 kB
57.797 kB
react
Spinner		 41.777 kB
14.478 kB
react
Stack		 41.734 kB
14.268 kB
react
Sticky		 32.613 kB
10.509 kB
react
Styling		 46.033 kB
15.135 kB
react
SwatchColorPicker		 190.945 kB
58.143 kB
react
TeachingBubble		 206.001 kB
61.059 kB
react
Text		 36.908 kB
12.822 kB
react
TextField		 80.816 kB
25.332 kB
react
Theme		 43.501 kB
14.183 kB
react
ThemeGenerator		 12.392 kB
4.126 kB
react
TimePicker		 241.905 kB
70.078 kB
react
Toggle		 46.225 kB
15.986 kB
react
Tooltip		 87.338 kB
28.223 kB
react
Utilities		 82.813 kB
25.115 kB
react
Viewport		 23.888 kB
7.656 kB
react
WeeklyDayPicker		 101.929 kB
31.829 kB
react
WindowProvider		 1.059 kB
541 B
🤖 This report was generated against 5bea3fa901c841e41438fff4b7ae6b38bf30b982

Mitch-At-Work
Mitch-At-Work approved these changes Oct 2, 2024
View reviewed changes
Copy link
Contributor

@Mitch-At-Work Mitch-At-Work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 629 634 5000
Breadcrumb mount 1700 1677 1000
Checkbox mount 1714 1712 5000
CheckboxBase mount 1491 1506 5000
ChoiceGroup mount 3012 2986 5000
ComboBox mount 685 686 1000
CommandBar mount 6542 6548 1000
ContextualMenu mount 12230 12261 1000
DefaultButton mount 807 822 5000
DetailsRow mount 2238 2296 5000
DetailsRowFast mount 2324 2230 5000
DetailsRowNoStyles mount 2053 2046 5000
Dialog mount 2866 2760 1000
DocumentCardTitle mount 236 239 1000
Dropdown mount 1995 2030 5000
FocusTrapZone mount 1187 1180 5000
FocusZone mount 1123 1097 5000
GroupedList mount 42763 42780 2
GroupedList virtual-rerender 20468 20355 2
GroupedList virtual-rerender-with-unmount 52206 52240 2
GroupedListV2 mount 235 233 2
GroupedListV2 virtual-rerender 209 210 2
GroupedListV2 virtual-rerender-with-unmount 224 249 2
IconButton mount 1175 1141 5000
Label mount 356 341 5000
Layer mount 2769 2847 5000
Link mount 412 402 5000
MenuButton mount 997 993 5000
MessageBar mount 21513 21486 5000
Nav mount 2053 2049 1000
OverflowSet mount 783 810 5000
Panel mount 1883 1910 1000
Persona mount 746 763 1000
Pivot mount 925 911 1000
PrimaryButton mount 938 941 5000
Rating mount 4691 4700 5000
SearchBox mount 931 938 5000
Shimmer mount 1938 1951 5000
Slider mount 1377 1342 5000
SpinButton mount 2964 3005 5000
Spinner mount 399 398 5000
SplitButton mount 1905 1888 5000
Stack mount 439 424 5000
StackWithIntrinsicChildren mount 868 897 5000
StackWithTextChildren mount 2852 2828 5000
SwatchColorPicker mount 6566 6523 5000
TagPicker mount 1528 1475 5000
Text mount 382 400 5000
TextField mount 938 957 5000
ThemeProvider mount 887 870 5000
ThemeProvider virtual-rerender 591 594 5000
ThemeProvider virtual-rerender-with-unmount 1298 1310 5000
Toggle mount 610 625 5000
buttonNative mount 190 201 5000

@spmonahan spmonahan merged commit b4e0624 into microsoft:master Oct 3, 2024
19 checks passed
mltejera pushed a commit to mltejera/office-ui-fabric-react that referenced this pull request Oct 3, 2024
@spmonahan @mltejera
fix: clear Modal timer on component unmount (microsoft#32969)
a898a74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabricteam fabricteam fabricteam left review comments

@Mitch-At-Work Mitch-At-Work Mitch-At-Work approved these changes

@sopranopillow sopranopillow sopranopillow approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
October Project Cycle Q4 2024
Development

Successfully merging this pull request may close these issues.

[Bug]: Error telemetry showing Firefox hitting NS_ERROR_UNEXPECTED on modal dialog
4 participants
@spmonahan @fabricteam @sopranopillow @Mitch-At-Work