Skip to content
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

remove the indeterminate class from Checkbox #4084

Closed
nicholasrice opened this issue Oct 29, 2020 · 7 comments
Closed

remove the indeterminate class from Checkbox #4084

nicholasrice opened this issue Oct 29, 2020 · 7 comments
Labels
bug A bug closed:obsolete No longer valid status:needs-information This issue needs more information warning:stale No recent activity within a reasonable amount of time
Projects

Comments

@nicholasrice
Copy link
Contributor

This issues tracks a follow-up to #4078.

Checkbox adds the indeterminate class to the custom element host when the control is in an indeterminate state. #4076 defines the intent of components to style themselves without hoisting classes to the element host.

This is problematic for the Checkbox because the Checkbox does not expose a indeterminate content attribute, only the indeterminate IDL attribute. The reason for this is parity with the <input type="checkbox" />.

There are a few options I can think of to address this:

  1. Add an indeterminate content attribute (are there any known downsides to this? Why didn't the platform do this?)
  2. Expose a IDL property that is a CSSStyleSheet that can be added / removed from the shadow root based on indeterminate state. This feels pretty un-ergonomic but it's an option
  3. Wait for the platform to add custom state to custom elements that can be selected for by CSS (Custom pseudo-classes for host elements via shadow roots (:state) WICG/webcomponents#738)
  4. Others?
@nicholasrice nicholasrice added status:triage New Issue - needs triage bug A bug status:needs-information This issue needs more information and removed status:triage New Issue - needs triage labels Oct 29, 2020
@EisenbergEffect EisenbergEffect added this to Triage in Components via automation Nov 2, 2020
@EisenbergEffect EisenbergEffect moved this from Triage to Backlog in Components Nov 2, 2020
@EisenbergEffect
Copy link
Contributor

@chrisdholt I wanted to draw your attention to this. This came up during our sync today with the Blazor folks. It would be nice to have this as an HTML attribute so that it can be set from HTML by Blazor. If it's only a property, Blazor can't set it and their customers would be forced to a JS interop layer for any checkbox they need to use this on, which isn't ideal.

@chrisdholt
Copy link
Member

chrisdholt commented Mar 24, 2021

@chrisdholt I wanted to draw your attention to this. This came up during our sync today with the Blazor folks. It would be nice to have this as an HTML attribute so that it can be set from HTML by Blazor. If it's only a property, Blazor can't set it and their customers would be forced to a JS interop layer for any checkbox they need to use this on, which isn't ideal.

Thanks @EisenbergEffect - curious if this is the case w/ standard inputs as well where this is still only an IDL attribute?

@EisenbergEffect
Copy link
Contributor

I think it's a property but not an attribute on built-ins. Per Nick's comment...this actually seems like an odd thing for the platform. We should probably bring it up with the HTML folks as something to address. In Blazor's case specifically, they can technically only call the setAttribute() API. So, if it's not an attribute, their customers can't set it without custom JS interop.

@chrisdholt
Copy link
Member

I think it's a property but not an attribute on built-ins. Per Nick's comment...this actually seems like an odd thing for the platform. We should probably bring it up with the HTML folks as something to address. In Blazor's case specifically, they can technically only call the setAttribute() API. So, if it's not an attribute, their customers can't set it without custom JS interop.

I'm good making this change, I think I'll also bring up an issue in Open-UI to elevate it there. Let's definitely track this as a larger HTML Platform improvement that's needed. Alternatively, they may push back and if so this seems like it would be an issue for Blazor all up. I'm good with making this change. We'll get it slotted.

@chrisdholt chrisdholt moved this from Backlog to To Do in Components Mar 24, 2021
@EisenbergEffect
Copy link
Contributor

EisenbergEffect commented Mar 24, 2021

There might be a similar type of issue with the listbox value or selectedindex as well. @nicholasrice is going to be working on the Blazor wrapper stuff and will be able to provide more details as he proceeds. All-in-all we think that working with Blazor is going to help us find little things like this that we can do to improve the components. Many of these fixes should also make things better for React developers as well.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the warning:stale No recent activity within a reasonable amount of time label Apr 16, 2022
@janechu
Copy link
Collaborator

janechu commented May 28, 2024

Unfortunately @microsoft/fast-components has been deprecated for some time.

@janechu janechu closed this as completed May 28, 2024
@janechu janechu added the closed:obsolete No longer valid label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug closed:obsolete No longer valid status:needs-information This issue needs more information warning:stale No recent activity within a reasonable amount of time
Projects
No open projects
Components
  
To Do
Development

No branches or pull requests

4 participants