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: Making Stack properly render non-ReactElement children #25685

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

khmakoto
Copy link
Member

PR Description

PR #25397 introduced a way to scope the Stack's child selectors so they wouldn't be as expensive for performance when recalculating styles. However, it seems that, in the process, we restricted Stack to work only with children that are React elements. This PR fixes the issue so that Stack works with any element (with the same restrictions to using enableScopedSelectors still in place).

Related Issue(s)

Fixes #25653

@khmakoto khmakoto added Component: Stack Fluent UI react (v8) Issues about @fluentui/react (v8) labels Nov 15, 2022
@khmakoto khmakoto requested review from a team as code owners November 15, 2022 22:07
@khmakoto khmakoto self-assigned this Nov 15, 2022
@khmakoto khmakoto enabled auto-merge (squash) November 15, 2022 22:10
@size-auditor
Copy link

size-auditor bot commented Nov 15, 2022

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-TeachingBubble 193.692 kB 193.689 kB BelowBaseline     -3 bytes
office-ui-fabric-react fluentui-react-Stack 40.056 kB 40.053 kB BelowBaseline     -3 bytes

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

Baseline commit: 4f5eec8081eed87fce6e8c4a071fb48e37629d1c (build)

@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 5efad2b:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Nov 15, 2022

📊 Bundle size report

🤖 This report was generated against 4f5eec8081eed87fce6e8c4a071fb48e37629d1c

@fabricteam
Copy link
Collaborator

fabricteam commented Nov 15, 2022

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 1417 1394 5000
Breadcrumb mount 3443 3495 1000
Checkbox mount 3148 3149 5000
CheckboxBase mount 2839 2803 5000
ChoiceGroup mount 5277 5261 5000
ComboBox mount 1475 1510 1000
CommandBar mount 11398 11212 1000
ContextualMenu mount 12941 13012 1000
DefaultButton mount 1640 1667 5000
DetailsRow mount 4269 4286 5000
DetailsRowFast mount 4394 4306 5000
DetailsRowNoStyles mount 4210 4177 5000
Dialog mount 3668 3721 1000
DocumentCardTitle mount 686 694 1000
Dropdown mount 3847 3769 5000
FocusTrapZone mount 2355 2381 5000
FocusZone mount 2353 2379 5000
GroupedList mount 2243 2557 2
GroupedList virtual-rerender 1334 1317 2
GroupedList virtual-rerender-with-unmount 1921 1972 2
GroupedListV2 mount 662 680 2
GroupedListV2 virtual-rerender 649 641 2
GroupedListV2 virtual-rerender-with-unmount 654 681 2
IconButton mount 2336 2306 5000
Label mount 864 861 5000
Layer mount 5150 5087 5000
Link mount 989 988 5000
MenuButton mount 1954 2016 5000
MessageBar mount 2594 2741 5000
Nav mount 3957 3860 1000
OverflowSet mount 1601 1597 5000
Panel mount 2994 3025 1000
Persona mount 1488 1509 1000
Pivot mount 1951 1948 1000
PrimaryButton mount 1818 1847 5000
Rating mount 8348 8311 5000
SearchBox mount 1771 1813 5000
Shimmer mount 3352 3472 5000
Slider mount 2439 2476 5000
SpinButton mount 5519 5567 5000
Spinner mount 939 957 5000
SplitButton mount 3758 3721 5000
Stack mount 968 1010 5000
StackWithIntrinsicChildren mount 2843 2829 5000
StackWithTextChildren mount 5799 5836 5000
SwatchColorPicker mount 13294 12341 5000
TagPicker mount 3070 3115 5000
TeachingBubble mount 98145 98744 5000
Text mount 928 939 5000
TextField mount 1866 1902 5000
ThemeProvider mount 1842 1834 5000
ThemeProvider virtual-rerender 1269 1269 5000
ThemeProvider virtual-rerender-with-unmount 2560 2602 5000
Toggle mount 1309 1282 5000
buttonNative mount 651 642 5000

@khmakoto
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@khmakoto khmakoto merged commit 9cd5e0a into microsoft:master Nov 15, 2022
@khmakoto khmakoto deleted the stackChildrenFix branch November 16, 2022 00:07
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
…t#25685)

* fix: Making Stack properly render non-ReactElement children.

* Adding change file.

Co-authored-by: KHMakoto <humberto_makoto@hotmail.com>
Hotell pushed a commit to Hotell/fluentui that referenced this pull request Feb 9, 2023
…t#25685)

* fix: Making Stack properly render non-ReactElement children.

* Adding change file.

Co-authored-by: KHMakoto <humberto_makoto@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Stack Fluent UI react (v8) Issues about @fluentui/react (v8)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: <Stack /> removes all non-ReactElement children while rendering
4 participants