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

[select] It's very unclear what actual API surfaces are introduced by this proposal #704

Closed
domenic opened this issue Mar 31, 2023 · 9 comments
Labels
select These are issues that relate to the select component stale

Comments

@domenic
Copy link
Contributor

domenic commented Mar 31, 2023

https://open-ui.org/components/selectmenu/#anatomy-of-the-selectmenu seems to imply the introduction of 4 new HTML elements: <button>, <selected-value>, <marker>, and <listbox>. It also lists the existing <option> and <optgroup> elements, and then it also lists the <select> element---probably that's a typo for <selectmenu>, so that would be a 5th new element?

And the diagram lists opt-group, not optgroup, so I'm not sure what's going on there.

But then if you check "show slots" we also get a button-container and listbox-container slot. And a few slots that have the same name as their child elements. And then an unnamed slot between listbox and opt-group. I guess this slot-versus-element issue is related to #702.

This explainer would significantly benefit from a clear listing of the proposed API surfaces, including:

  • New elements
  • New special content attributes (behavior=""??) or ways of modifying the behavior of existing content attributes (slot="")
  • What special values the content attributes can have
@josepharhar josepharhar added the select These are issues that relate to the select component label Apr 6, 2023
@josepharhar
Copy link
Collaborator

https://open-ui.org/components/selectmenu/#anatomy-of-the-selectmenu seems to imply the introduction of 4 new HTML elements: <button>, <selected-value>, <marker>, and <listbox>. It also lists the existing <option> and <optgroup> elements, and then it also lists the <select> element---probably that's a typo for <selectmenu>, so that would be a 5th new element?

And the diagram lists opt-group, not optgroup, so I'm not sure what's going on there.

But then if you check "show slots" we also get a button-container and listbox-container slot. And a few slots that have the same name as their child elements. And then an unnamed slot between listbox and opt-group. I guess this slot-versus-element issue is related to #702.

Hopefully #710 addresses all of this

New special content attributes (behavior=""??)

I agree we should have a section on the behavior attribute, but if we use actual elements like you mentioned in #702 and #703 then the behavior attribute would go away so I'll hold off for a little bit

ways of modifying the behavior of existing content attributes (slot="")

I don't see how we are modifying the existing behavior of the slot attribute

@domenic
Copy link
Contributor Author

domenic commented Apr 6, 2023

Right now slot="" is used for assigning to web developer defined slots. Redefining it to slot into some sort of user agent internal slot is a behavior change. (E.g. it changes how a selectmenu inside a shadow tree with its own slots would behave.)

@gregwhitworth
Copy link
Member

New special content attributes (behavior=""??)

Yep, I think this is a key thing that we need to land ASAP. We do not intend on introducing new HTML elements for all parts of components/controls that we recommend introducing unless they make sense on their own. So the key thing we need specified here is the behavior content attribute and how this wires up the functionality.

@josepharhar note that some of this content is in our very early draft for customizable controls here although we moved away from using the part attribute (I'll need to track down the issue for why we needed a new one and couldn't overload it for use in HTML).

Right now slot="" is used for assigning to web developer defined slots. Redefining it to slot into some sort of user agent internal slot is a behavior change. (E.g. it changes how a selectmenu inside a shadow tree with its own slots would behave.)

Then this is correct in that we are recommending a behavior change, thanks for calling it out.

@domenic
Copy link
Contributor Author

domenic commented Apr 10, 2023

We do not intend on introducing new HTML elements for all parts of components/controls that we recommend introducing unless they make sense on their own.

My point is precisely that this intention of yours is counter to the design of HTML so far, and so I recommend against it, if you want to land on something that will fit with browser implementations and the HTML Standard.

@gregwhitworth
Copy link
Member

My point is precisely that this intention of yours is counter to the design of HTML so far, and so I recommend against it, if you want to land on something that will fit with browser implementations and the HTML Standard.

This becomes priority number one for us to solve then because this is a key pillar to enable extensibility of the controls/components being defined while allowing them to not have to duplicate all of the functional capabilities.

@github-actions
Copy link

github-actions bot commented Oct 8, 2023

There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label.

@github-actions github-actions bot added the stale label Oct 8, 2023
@josepharhar
Copy link
Collaborator

I think that the current state of the explainer answers all of these questions. I replaced the behavior attribute with new elements and listed them all out.

I suppose that <listbox> could be defined in more detail, but I currently have my own explainer for that for now: https://github.com/josepharhar/explainers/blob/main/listbox.md

@lukewarlow
Copy link
Collaborator

@josepharhar is it worth copying that across to an openui mdx file? Just so it can get updated to latest things and be referenced during discussions? (e.g. it says :selected but we decided to stick with :checked for now, and the content model section seems to contradict how it works in selectlist)

@josepharhar
Copy link
Collaborator

is it worth copying that across to an openui mdx file

Yes! lets talk about it here: #896

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
select These are issues that relate to the select component stale
Projects
None yet
Development

No branches or pull requests

4 participants