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: How should we render <option label=label>text? #1115

Open
josepharhar opened this issue Oct 23, 2024 · 8 comments
Open

select: How should we render <option label=label>text? #1115

josepharhar opened this issue Oct 23, 2024 · 8 comments
Labels
select These are issues that relate to the select component

Comments

@josepharhar
Copy link
Collaborator

In the pre-customizable-select chromium implementation, option elements only render text content by manually collecting it from their descendants. If there is a label attribute, then the text cotent of descendant nodes is ignored and the text in the label attribute is rendered instead.

There is currently some code in chromium which tries to preserve this rendering behavior when not inside a customizable select, but doesn't do this label overwriting behavior and allows all child nodes to be rendered normally when inside a customizable select. It's not very performant right now and I'm currently trying to find ways to work around it.

It occurred to me that this has never been discussed before and could affect how I implement this - how should we render <option label=label>text</option>? Should "label" or "text" be rendered?

This would also affect cases like <option label=label><img src=...><span>text</span> more text</option> - if we choose to render "label", then the img and other child content would not be rendered.

If we don't render the label attribute, then should that label attribute be used in the accessibility tree instead of the actual child nodes? Is there a use case for that? @scottaohara

Related: #558

@josepharhar josepharhar added select These are issues that relate to the select component agenda+ Use this label if you'd like the topic to be added to the meeting agenda labels Oct 23, 2024
@josepharhar
Copy link
Collaborator Author

This issue provides a use case for not using the label attribute when rendering: #1118

@josepharhar
Copy link
Collaborator Author

Right now the label attribute is not used for rendering, but does get copied into the default/fallback button, which is probably not good and should be fixed after we decide what to do here.

<select>
  <option label=label>text</option>
</select>

_usr_local_google_home_jarhar_Downloads_label html

@josepharhar
Copy link
Collaborator Author

In the spec PR, I'm already saying that options don't use the label attribute to render: https://whatpr.org/html/10629/rendering.html

An option element is expected to be rendered by displaying the element's label, indented under its optgroup element if it has one. If the option is being rendered in a select which is being rendered as a drop-down box with base appearance, then the option is expected to render all of its children rather than by displaying its label.

@josepharhar
Copy link
Collaborator Author

Proposed resolution: Don't use the option's label attribute to render anything anywhere in the new select.

@sorvell
Copy link

sorvell commented Oct 31, 2024

It seems unfortunate to change this behavior from how non-custom select works.

Is there another case besides #1118 that needs a label distinct from the children?

Is label relevant to a11y or would that be aria-label and if it's the latter, then is the keyboard case in #1118 an a11y concern and could it therefore use aria-label? (Unfortunately, it looks like aria-label isn't used for keyboard navigation in the normal select)

@scottaohara
Copy link
Collaborator

scottaohara commented Oct 31, 2024

label is not an a11y-specific attribute, in that it was a method to provide a visible label, rather than using nested text, for an option.

aria-label aside, the accName of the option either comes from the label - which produced visible text 'in' the option, or the visible text an author provides within the option. so that's why it's not an a11y-feature, because they were two methods to render a visible label. previously, the label attr took precedent over the visible text, so the visible text an author provided would be ignored, and the label text would be present within the option. but for calculating the accname, that visible text was all that mattered (again, aria-label aside and the fact it wasn't even consistently used by all browsers/AT to modify an accName - since the UA provided options weren't necessarily 1:1 matches with the markup that someone wrote) but in the context of a datalist, an option can have visible text AND a label attribute's text both made visually available in an option. I've mentioned in the current select PR that this distinction is lost with the updates to the content model for options. arguably, this similar behavior could be useful for the new customizable select too...

@scottaohara
Copy link
Collaborator

linking this issue to my HTML AAM PR for option element mapping updates, - w3c/aria#2365

@css-meeting-bot
Copy link

The Open UI Community Group just discussed select: How should we render `<option label=label>text`?, and agreed to the following:

  • RESOLVED: render the label attribute for options when it is present, regardless of the select's appearance value, just like it already does. also the label attribute is bad.
The full IRC log of that discussion <gregwhitworth> jarhar: the option label has the label attribute and in the customizable select world this attribute where you have <option label=label>text and it will render label and not text
<gregwhitworth> jarhar: in the new customizable select world I implemented a thing that ignores the label attribute as I find it weird
<gregwhitworth> jarhar: how should this attribute work, scott says that it should still be used
<scott> q+
<gregwhitworth> jarhar: given this is what it already does it says "render the label attribute unless your the child of a customizable select"
<gregwhitworth> jarhar: also sarah left a comment about typeahead approach and the more I think about this to change the behavior of label is to keep the old behavior
<gregwhitworth> q?
<gregwhitworth> ack scott
<gregwhitworth> scott: thank you for the summary
<sorvell> q+
<masonf> q+
<gregwhitworth> scott: the latest comment I posted is in reviewing the PR yesterday I was getting tripped up with some of the simplication as it does something different if it is in a datalist and it will show it as additional text and in Chrome that actual additional text can be used as a hint and in other browsers it doesn't show
<sarah> q+
<gregwhitworth> scott: I would love if we don't do current behavior I would love to follow suit with datalist and possibly fix this across them
<gregwhitworth> scott: those are my thoughts on it
<gregwhitworth> ack dbaron
<gregwhitworth> dbaron: I wanted to mention the history of this is that it may have gotten lost
<gregwhitworth> dbaron: the reason label exists is that label and optgroup were introduced at the same time and it exists for browsers that don't support optgroup
<gregwhitworth> dbaron: the options of the label attribute if you don't support optgroup but if you do then you show optgroup
<gregwhitworth> dbaron: given that we don't have to worry about compat with optgroup that doesn't say a lot of what we should do
<gregwhitworth> dbaron: the whole plan got messed up by not implementing the same things at the same time
<gregwhitworth> dbaron: since it was the idea and browsers would implement each feature 5 years apart
<gregwhitworth> q?
<gregwhitworth> ack sorvell
<gregwhitworth> sorvell: there as some stuff for closing tags for option and you could use the label; this all seems legacy
<gregwhitworth> sorvell: I think it will be hard to spec how the label will work and the custom select is meant to control how things are rendered
<gregwhitworth> sorvell: I don't think it's 100% orthogonal to that as it does seem pretty necessary to cover in MVP
<gregwhitworth> ack masonf
<gregwhitworth> masonf: I was there primarily to discuss the typeahead of that. We say that we should render the label and people will stop using it as it will not render the way they expect it to
<gregwhitworth> masonf: it's related but orthogonal to typeahead and I would want to ignore what's there and what you search for
<gregwhitworth> q+
<gregwhitworth> masonf: do what the old thing is supported
<gregwhitworth> ack sarah
<gregwhitworth> sarah: When I was writing the typeahead issue we had discussed this but knowing the issue is label should stay odd due to history
<gregwhitworth> sarah: but if it stays there then people will need to know that when it appears odd they'll adjust
<dbaron> s/the reason label exists/I *think* the reason label exists/
<gregwhitworth> sarah: I also proposed the text attribute as the label has meaning around that and this wouldn't contribute to the accessible name and it may contribute to the value but not accname
<gregwhitworth> q?
<masonf> gregwhitworth: I worked with Sunir on the combobox, it'd be cool if we could fork into a new issue for the `text` attribute. We're all agreeing otherwise.
<jarhar> proposed resolution: render the label attribute for options when it is present, regardless of the select's appearance value, just like it already does. also the label attribute is bad.
<gregwhitworth> ack gregwhitworth
<sorvell> +1
<masonf> +1
<gregwhitworth> +1
<scott> +1
<dbaron> +1
<sarah> +1
<jarhar> RESOLVED: render the label attribute for options when it is present, regardless of the select's appearance value, just like it already does. also the label attribute is bad.
<sarah> this is the other issue about the typeahead / maybe value-related question: https://github.com//issues/1118

@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Nov 6, 2024
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
Projects
None yet
Development

No branches or pull requests

5 participants