-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
chore: refactor Flex to be functional component [v0] #12078
Conversation
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 3d5fba42e42c72fe33dd2ce54cdda2e1cd940835 (build) |
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Potential regressions comparing to master
Perf comparison
Perf tests with no regressions
|
children: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), | ||
|
||
align: PropTypes.oneOf(['auto', 'start', 'end', 'center', 'baseline', 'stretch']), | ||
size: PropTypes.oneOfType([PropTypes.oneOf(['size.half', 'size.quarter', 'size.small', 'size.medium', 'size.large']), PropTypes.string]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously was incorrect 💣
…OfficeDev/office-ui-fabric-react into chore/flex-to-fc � Conflicts: � packages/fluentui/CHANGELOG.md
static defaultProps = { | ||
as: 'div' | ||
}; | ||
const content = React.Children.map(children, child => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's refactor this with adding FlexContextProvider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, can we do this thing separately? I want to make a small showcase from it
children: PropTypes.oneOfType([PropTypes.element, PropTypes.func]), | ||
|
||
align: PropTypes.oneOf(['auto', 'start', 'end', 'center', 'baseline', 'stretch']), | ||
size: PropTypes.oneOfType([PropTypes.oneOf(['size.half', 'size.quarter', 'size.small', 'size.medium', 'size.large']), PropTypes.string]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit worried that this can be any css value (px or %) which may cause problems with the caching... :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see the feedback. I agree there that it can be a real issue, but:
- I don't think that it's used enough frequently
size
managesflexBasis
, may be in this case we should manage it viadesign
prop...- we can disable caching for
FlexItem
…c-react into chore/flex-to-fc � Conflicts: � packages/fluentui/CHANGELOG.md
Pull request checklist
$ yarn change
BREAKING CHANGES
This PR converts
Flex
&FlexItem
components to be functional. As in all other cases it contains a breaking change with restricted props set that will be passed to styles functions.Prop sets
Flex
column
debug
fill
gap
hAlign
inline
padding
space
vAlign
wrap
FlexItem
align
grow
flexDirection
push
shrink
size
Microsoft Reviewers: Open in CodeFlow