feat: add defaultindeterminate attribute to checkbox for setting indeterminate declaratively - #36514
Conversation
…terminate declaratively
26a742a to
16f65cf
Compare
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-web-components/Accordion 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Accordion. - Dark Mode.normal.chromium_1.png | 3398 | Changed |
| * | ||
| * @internal | ||
| */ | ||
| private dirtyIndeterminate: boolean = false; |
There was a problem hiding this comment.
is dirtyIndeterminate needed? Couldn't it just piggyback onto dirtyChecked?
There was a problem hiding this comment.
We might be able to.
| } | ||
|
|
||
| this.dirtyIndeterminate = true; | ||
| toggleState(this.elementInternals, 'dirty-indeterminate', true); |
There was a problem hiding this comment.
Is there a true need for a dirty-indeterminate state? We don't have a separate state for dirty-checked.
There was a problem hiding this comment.
There is a reason for this, and I think it's one-of-a-kind situation (maybe I missed something simpler). Consider the lifecycle of defaultindeterminate.
- A server-rendered indeterminate means we hook off the attribute for initial visual styling (avoid using states which should be client-side only). The checkbox appears as indeterminate.
- Click the checkbox => checkbox shows checked and no longer indeterminate, attribute styling is preserved for
defaultindeterminate - Click the checkbox again => checkbox is unchecked, attribute styling is preserved for
defaultindeterminate
We need to preserve the attribute that's passed from the client (we should not remove this or toggle upon user interaction). We need a way to essentially say, "This is no longer indeterminate" while preserving the attribute from user-land. I chose a state here because it explicitly applies client-side.
There was a problem hiding this comment.
I'm open to naming on the state, but absent another solution it seems the most elegant to address what is purely a styling problem post-user-interaction.
| * @remarks | ||
| * HTML Attribute: `defaultindeterminate` | ||
| */ | ||
| @attr({ attribute: 'defaultindeterminate', mode: 'boolean' }) |
There was a problem hiding this comment.
Why defaultindeterminate? We don't use defaultchecked or defaultdisabled as attribute names. Since indeterminate doesn't yet exist as an attribute, can't that be used, while still mapping it to the defaultIndeterminate property?
There was a problem hiding this comment.
Because reusing indeterminate is not the direction the platform is most likely to go. indeterminate would suffer the same issue that checked does where there is increased cognitive load and confusion as it only pertains to the initial state. See the discussion here which is referenced in the issue: whatwg/html#6578
microsoft-github-policy-service
Bot
added
Needs: Author Feedback
and removed
Needs: Author Feedback
labels
2 participants
Previous Behavior
On the HTML Platform there is currently no way to have
indeterminateexist at runtime without hydration, this issue also exists with the current Fluent checkbox.New Behavior
This PR proposes a new
defaultindeterminateattribute to support setting indeterminate declaratively. The proposed approach follows the discussion and direction provided in whatwg.