-
Notifications
You must be signed in to change notification settings - Fork 164
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
Experimental v2 component framework #335
Conversation
For comparison bundle sizes for the relevant portions of the frameworks look like:
|
color?: string; | ||
|
||
/** alternate mode for disabled look and feel */ | ||
disabled?: TextTokens; |
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.
I'm unsure what this is for. Is it used to define a style for disabled? I'm confused why TextTokens is used in the definition of TextTokens.
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.
Yes, this is to define a style for disabled. The reason it is recursive (as a pattern) is that if there are multiple states they each can contain the other ones. (This way you can do hovered + pressed, or primary + disabled, if those are states you have.)
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.
This is equivalent to what before was:
settings: {
_overrides: {
disabled : {
tokens: {
// same content here
}
}
}
}), | ||
buttonName | ||
], | ||
states: ['hovered', 'focused', 'pressed', 'disabled'], |
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.
This replaces __overrides, right? How does this change for nested states (focused + pressed or whatever)
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.
I just saw your response to Krystal elsewhere, this is still recursive
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.
These changes look good to me. I think it definitely simplifies everything and makes component creation easier to understand. Great work!
Platforms Impacted
None and all. This change should be looked at like a proposal. It is an alternate component framework that is smaller, lighter, hopefully easier to use and understand, and potentially a bit better aligned with where web is going. Feedback is welcome and because it is an experimental section of code it can be iterated on after merge as well.
Description of changes
This reworks the component building system, in particular the foundation-composable, foundation-tokens, themed-settings, most of foundation-settings, and foundation-compose packages. It then creates four new packages, which provide a parallel experimental way of building components.
It also includes reworking Button, Text, and Stack to use the new framework. The packages are as follows:
@fluentui-react-native/use-styling
This is used almost identically to the useStyling function passed into
usePrepareProps
, this module creates a generic way of building an opinionated styling hook and puts it in its own package. This has a number of changes from the way the current system is set up:_overrides
with the precedence array in the settings, the layers/states are just directly included in the tokens, and the precedence becomes a direct setting for the component.I did write a doc file for this one included in this PR. It can also be found at: https://github.com/JasonVMo/fluentui-react-native/tree/experimental-framework/packages/experimental/use-styling
'@fluentui-react-native/use-slots`
This reworks the slots mechanism into a more focused and reusable package. I don't have a README.md written for this yet but it effectively looks like:
buildUseSlots
This can attach styling, or not, all parameters to useSlots get passed to the styling hook, if set, then those properties bubble back out.
stagedComponent
The other thing included in here is a new pattern for splitting hooks and rendering. This is necessary for tree compression and is the reason why the framework today has
usePrepareProps / render
. The issue with the current pattern is that it is awkward to pass information from usePrepareProps to render, the whole IRenderData and state are expressly for that purpose. It also is a bit too tied to the framework. Instead we can solve the same problem by writing components as follows:stagedComponent
wraps any function of this form into a traditional component while also exposing an attached reference to the original staged component for aware frameworks.'@fluentui-react-native/composition`
This package effectively implements a generic compose called
composeFactory
and adds some of the type extraction goodness that the previous compose used. This is pretty small and is primarily a packager for the other two implementations.`@fluentui-react-native/experimental-framework'
This rolls the packages above together, and adds the opinion on how theming works. For generics parameterized on the type of the theme this package re-exports them to make them opinionated.
Components (Button, Text, Stack)
This is probably the best place to look for how the new code functions in practice. Existing tests have been ported and run against the previous snapshots.
Verification
(how the change was tested, including both manual and automated tests)
Pull request checklist
This PR has considered (when applicable):