fluentui-blazor
fluentui-blazor copied to clipboard
[Card] Fluent UI 2 Card and FocusManager
Pull Request
📖 Description
-
Optional Fluent UI 2 Card look and behavior, it's activated when FluentCardHeader, FluentCardPreview or FluentCardFooter is added to card's content. Implementation is a Blazor replica of Card component from Fluent UI React. Card's CSS uses v2 design tokens by default and v1 tokens as fallback.
-
FocusManager - helps to control focus behavior, it's basically a wrapper for tabster which is used in Fluent UI React and Fluent UI Web Components v3. Added because of Card's FocusMode.
-
Rendering of
tabindex
attribute was added to FluentButton - libraries, including tabster, usually use 'standard' query selector to find focusable elements. This selector includes standard elements likea
,input
,button
etc., and elements with tabindex attribute, as a result Fluent UI Web Components can be programmatically identified as focusable only if they havetabindex
attribute.
✅ Checklist
General
- [x] I have added tests for my changes.
- [x] I have tested my changes.
- [x] I have updated the project documentation to reflect my changes.
- [x] I have read the CONTRIBUTING documentation and followed the standards for this project.
Component-specific
- [ ] I have added a new component
- [ ] I have added Unit Tests for my new compontent
- [x] I have modified an existing component
- [x] I have validate Unit Tests for an existing component
I found this PR very very large (141 updated files), probably because there are several features included in this one PR 🤪
Give us a few days to check and verify all your code.
Probably, half of these files are *.verified.html files - most of them are files affected by 'tabindex' added for fluent-button, other are new tests for Card)
Yes. I saw that.
Is it possible to avoid inserting a default tabindex
in existing components? But only when the developer wants to include this new functionality?
I would prefer not to modify the generated code of all the components, which could be considered as a breaking change.
Yeah, having it rendered all the time was bugging me as well... From the top of my head, the following could be done to mitigate this issue:
- add
EmitTabIndexAttribute
property toLibraryConfiguration
- add
internal static bool EmitTabIndexAttribute = false;
toFluentComponentBase
- add
FluentComponentBase.EmitTabIndexAttribute = options.EmitTabIndexAttribute;
toServiceCollectionExtensions.AddFluentUIComponents
- modify
FluentComponentBase.GetTabIndexAttributeValue
to return null ifEmitTabIndexAttribute
is set tonull
-tabindex
won't be rendered in this case This is `opt-in' for the whole app. What do you think about this approach or maybe you have something else on your mind?
That sounds like a solid approach.
I'm a bit worried about using/including the tabster package as the repo mentions it is all a bit experimental.
Another thing is I do not 'just' want to blindly copy Fluent React components. We tried that before with the BlazorFluentUI library and we just could not keep up. I need to check with the fluent team to see what the plans are for the Card web component. Not shooting this down but let's take some time to do it right (so it will not be part of our next release (4.3))
Re tabster, yes, it says it's still WIP PoC, but Fluent React is using it for [Focus Management] (https://react.fluentui.dev/?path=/docs/utilities-focus-management-usearrownavigationgroup--default) and it gave a courage to try it 🙃
Re WC v3, i was monitoring the activity in v3 branch for the past six month - besides the spike of commits related to unit tests, there were no significant changes. Basically, that's why i decided to port Card.
Re WC v3, i was monitoring the activity in v3 branch for the past six month - besides the spike of commits related to unit tests, there were no significant changes. Basically, that's why i decided to port Card.
Understood. Nevertheless I will check with the team. We are trying to get all Fluent activities closer aligned and I believe in the end that will be better for everyone.
Was looking at this again and I now see for the WC v3 they are using "tabbable": "^6.2.0". Think it makes sense for us to use that then as well
I'm closing this one for 2 reasons.
- This uses a different helper js script (tabster/tabbable)
- We intruduce Fluent2 design for 1 specific component
I would still be interested in getting the FocusManager/tabindex work in, but think it is better to have a separate PR for that (and using the new js helper)