Checkbox spec: clarifying conversion table & adding work items to track conversion. #2212
Conversation
…le more clear, adding work items to track conversion of current Fluent/Stardust to new Fluent Checkbox.
aneeshack4
requested review from
dvdzkwsk,
dzearing,
ecraig12345,
JasonGore,
jdhuntington,
kenotron,
layershifter,
levithomason,
miroslavstastny and
mnajdova
as code owners
Perf comparison
Generated by 🚫 dangerJS |
| @@ -156,9 +129,10 @@ Removing the following two props because the ARIA spec dictates role='checkbox' | |||
| | `ariaLabelledBy` | User provided | ❌ | ❌ | ❌ | | |||
| | `ariaPositionInSet` | Won't be transitioned| ❌ | ❌ | ❌ | | |||
| | `ariaSetSize` | Won't be transitioned| ❌ | ❌ | ❌ | | |||
There was a problem hiding this comment.
I believe all aria* attributes we weren't going to carry over the camelCased variants and instead use aria-* native attributes.
There was a problem hiding this comment.
The most recent thing I remembered (from before the holidays) was the opposite, using camelCased instead of native.
There was a problem hiding this comment.
as the acccessibility behaviors are platform independent, we opted for using aria-* initially istead of camel case. camel case is react specific and having just one approach in both components and behaviors brings consistency. @ecraig12345 it would be good to cover these decisions in design principles, so that we can refer to them and discsus them - I see pros and cons of this but the dicsussion might be lost when done under checkbox.
| - [ ] Decide if `onClick` is too similar to `onChange` and if so, remove it. If not, change its type to native instead of the current `ComponentEventHandler`. | ||
| - [ ] Remove `toggle` - it will be a separate variant component of Checkbox. | ||
| - [ ] Remove `variables` - theming will be handled differently in the new Fluent. | ||
| - [ ] Remove `design` prop. |
There was a problem hiding this comment.
@levithomason was saying there were reasons to keep this that you all discussed in the second week, I thought.
There was a problem hiding this comment.
To keep the design prop? I don't recall or see anything about it on the hackmd notes.
| - [ ] Remove `variables` - theming will be handled differently in the new Fluent. | ||
| - [ ] Remove `design` prop. | ||
| - [ ] Decide on the TBD props `as`, `styles`, and `theme` which will apply to other components across the new Fluent library. | ||
| - [ ] Keep `labelPosition`, `checked`, `defaultChecked`, `className`, and `disabled` props unchanged. |
There was a problem hiding this comment.
The docs don't really mention that unrecognized native "div" (?) props should be mixed into root. I believe we expected that to continue to work.
There was a problem hiding this comment.
Do you mean the spec & not the docs? It's mentioned like this under the screenreader accessibility section but I'll add it here too:
should mix in native props expected for the element type defined in
as.
|
|
||
| ### Work items and tracking progress on conversion from the current Fluent (Stardust) checkbox to the new Fluent checkbox we're building with this spec: | ||
| - [ ] Remove `animation` prop. | ||
| - [ ] Remove `accessibility` prop. |
There was a problem hiding this comment.
as discussed on the sync last week, we should not remove accessibility prop. We can have a separate discsussion about a replacement for it
There was a problem hiding this comment.
Ah I wasn't aware of this - what is this separate plan/is it documented anywhere?
We arrived at the final props list in this conversion table by comparing the props lists of the current Fabric and Stardust Checkboxes and the conversion was meant to track the progress of the conversion from the Fabric Checkbox, not the current Stardust/Fluent Checkbox in this repo. So added some clarification of that and added a list of work items to track the conversion of the current Stardust/Fluent Checkbox to the new Fluent Checkbox we're trying to build with this spec.