-
Notifications
You must be signed in to change notification settings - Fork 2.9k
docs: behavior state hooks RFC #35623
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
base: master
Are you sure you want to change the base?
docs: behavior state hooks RFC #35623
Conversation
227bbb8 to
948a0d2
Compare
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
docs/react-v9/contributing/rfcs/react-components/convergence/behavior-state-hooks.md
Outdated
Show resolved
Hide resolved
|
|
||
| ### Can I mix behavior state hooks with styled components? | ||
|
|
||
| Yes! You can use behavior state hooks for some components and styled components for others in the same application. Keep in mind you'll need to provide styles when using behavior state hooks. |
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! You can use behavior state hooks for some components and styled components for others in the same application. Keep in mind you'll need to provide styles when using behavior state hooks. | |
| Yes. | |
| > [!NOTE] | |
| > We recommend to stick to a single option in your application to avoid UI misalignments. | |
| You can use behavior state hooks for some components and styled components for others in the same application. Keep in mind you'll need to provide styles when using behavior state hooks. |
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.
lets make this suggestion as required rather "suggested change" as it's critical piece of information
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.
Why do you feel it needs to be required? You can still use it that way, and there’s already a note with recommendations and implications. Making it “required” sounds like we’re saying you can’t mix different approaches, which is not technically correct to say
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 don't think we should make it required (maybe strongly suggested?). My reason is that I think we ultimately need to be able to support a migration path if folks have an app with the existing, styled Fluent components and want to move to behavior hooks. I don't think we need to have this 100% sorted out now, just need to leave the door open for it.
| ### Why the `_unstable` suffix? | ||
|
|
||
| The `_unstable` suffix is consistent with Fluent UI's convention for hooks. While behavior state hooks are meant to be used in production, the suffix indicates they're lower-level primitives whose implementation details may evolve. | ||
|
|
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.
IMO there should be Out of scope section that covers render functions. I see that they are used in the example above, but as we discussed components that use portals might be challenging.
### Out of scope
#### Render functions
Base hooks provide only behavior (state, accessibility handling, etc.) while to build actual components you will also need render functions to product markup.
Our general recommendation at the moment is to re-use existing render functions rather then implementing custom. We are going to provide more guidance in the future RFC for Unstyled components.
??
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.
Aside: hopefully we can minimize the need for portals as things like Popover API and CSS anchor positioning get better support.
26542d6 to
82bd448
Compare
| Introduce behavior state hooks per component using the naming pattern `use${ComponentName}Base_unstable`: | ||
|
|
||
| ```tsx | ||
| import { useButtonBase_unstable } from '@fluentui/react-components'; |
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.
for pilot testing/usage I think we discussed to stick with per-package only ?
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 open to feedback on this - both options should work. If no one has strong arguments or pushback against it, we can go ahead and add it to the suite too.
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.
@khmakoto Do you have an opinion here?
docs/react-v9/contributing/rfcs/react-components/convergence/behavior-state-hooks.md
Outdated
Show resolved
Hide resolved
| const { appearance = 'secondary', size = 'medium', shape = 'rounded' } = props; | ||
|
|
||
| return { | ||
| ...useButtonBase_unstable(props, ref), |
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.
briefly touched offline, but have we thought about how we will enforce that the "proper" logic lives in the proper domain -> base vs non base?
this might be crucial to not deviate from the domain of the behaviour hook
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.
We don’t really enforce anything with existing state hooks - for example, everyone agrees not to put Griffel styles in a state hook, but there’s no actual rule about it. So, what makes you think we should enforce it in this case?
|
|
||
| ## Usage Example | ||
|
|
||
| Building a custom button with your own design system: |
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.
might be worth to actually provide "Real example" containing custom provider and tokens ( as we are mentioning "design system" )
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 intentionally didn't add that, as I want to avoid 20+ comments about personal opinions on how it should be written/implemented 😄. I'd leave this for users to decide how to setup tokens and as it really depends on the styling approach.
docs/react-v9/contributing/rfcs/react-components/convergence/behavior-state-hooks.md
Outdated
Show resolved
Hide resolved
docs/react-v9/contributing/rfcs/react-components/convergence/behavior-state-hooks.md
Outdated
Show resolved
Hide resolved
|
|
||
| ### Do behavior state hooks guarantee accessibility? | ||
|
|
||
| Behavior state hooks provide correct ARIA attributes, keyboard handling, and semantic structure. However, **you're responsible for ensuring custom styles don't interfere with accessibility (e.g., sufficient contrast, visible focus indicators).** |
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.
could this be provided as lint rules/conformance test suite for these adopters to make sure a11y is not broken when they decide to use this api ?
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.
that's a good point, but tbh I'm not sure we're in a position/have capacity for this. and also don't think we should, we definitely should provide a guidance/recommendation, but we can't enforce something that is beyond of ours control.
|
|
||
| ### Can I mix behavior state hooks with styled components? | ||
|
|
||
| Yes! You can use behavior state hooks for some components and styled components for others in the same application. Keep in mind you'll need to provide styles when using behavior state hooks. |
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.
lets make this suggestion as required rather "suggested change" as it's critical piece of information
| ```tsx | ||
| import { useButtonBase_unstable } from '@fluentui/react-components'; | ||
| // or from the package directly | ||
| import { useButtonBase_unstable } from '@fluentui/react-button'; |
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.
Based on the name of this RFC I'll suggest these hooks might be named use$ComponentNameBehavior_unstable --> useButtonBehavior_unstable.
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.
One of the other main things we'd need to remove in the use$ComponentNameBehavior_unstable hooks would be things like tabster (useArrowNavigationGroup, etc.).
imo I'd expect that to be included in the "Behaviour" hook based on the name but would need to be excluded in practice.
Blurs the line a bit with a11y too as often the keyboarding model would be considered part of that but we'd actively want to exclude it from this new hook
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.
One of the other main things we'd need to remove in the
use$ComponentNameBehavior_unstablehooks would be things like tabster (useArrowNavigationGroup, etc.).imo I'd expect that to be included in the "Behaviour" hook based on the name but would need to be excluded in practice.
Blurs the line a bit with a11y too as often the keyboarding model would be considered part of that but we'd actively want to exclude it from this new hook
That’s not going to work for this version since tabster is still pretty important for the a11y setup. We can’t just leave it out of the hook right now. We should put together a separate proposal and tackle it on its own. We're also still need to figure out if we want to drop/replace it, or maybe just make it lazy-loadable.
|
|
||
| ### What about accessibility-critical inline styles? | ||
|
|
||
| Default behavior state hooks should not apply styles. The exception is when a specific CSS property is required for accessibility or core functionality (and is currently implemented via Griffel styles). In those cases, apply the minimum inline style needed and document the rationale. |
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.
In those cases, apply the minimum inline style needed and document the rationale.
Will this create specificity problems? Inline styles have fairly high specificity.
Perhaps it won't much matter in practice as I imagine overriding these styles is generally a bad idea but I think we should have an example of how an override might work here.
| Yes. You can use behavior state hooks for some components and styled components for others in the same application. | ||
|
|
||
| > [!NOTE] | ||
| > We recommend sticking to a single option in your application to avoid UI misalignments. |
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 with this recommendation but I think we should have a notion of what it looks like for an app to migrate from styled Fluent to behavior state hooks.
|
@dmytrokirpa in offline we discussed risks of shipping new hooks in the stable packages.
I don't have strong opinions on this (except that we shouldn't re-export them from the suite package from day 1), so feel free to propose options based on the discussion yesterday. |
@layershifter Updated the behavior-state-hooks RFC with several key changes: 1. Naming Convention Change:
2. Exports and Naming Section (new):
3. Release Strategy Section (new):
Commit: 52a0480 |
What’s in this PR
This PR adds the RFC
“Component Behavior State Hooks”as a separate proposal.It is extracted from the broader
“RFC: Unstyled Components”work in #35464, so reviewers can discuss the behavior-state-hooks proposal independently of the other changes in that PR.The original PR also contains additional context and illustrative examples (including related patterns explored alongside unstyled components). If you want to see the broader exploration and code samples, refer to the linked PR.
What’s not in this PR
No implementation changes; this is documentation/RFC only.