Define base appearance for select multiple drop-down#12069
Conversation
|
This needs additional changes to explain that |
|
The CSS Working Group just discussed |
|
I have made the changes I mentioned in my last comment regarding changing the drop-down box definition. This is ready for review. |
| <p>If <var>select</var> has the <code data-x="attr-select-multiple">multiple</code> | ||
| attribute, then return.</p> |
There was a problem hiding this comment.
This seems kind of tricky to test since the resulting text that is rendered is explicitly not defined. In chrome I know that when a second option becomes selected, the resulting text that gets rendered is "2 selected", and I could do a mismatch reftest against the text being "option 2", but it would technically be legal with this text for another implementation to write "option 2" here if they wanted.
What do you think? Do you think we should test that anyway, or do you have any other ideas of how to test this?
There was a problem hiding this comment.
Hmm I think instead of saying return here, it should say "set select's select fallback button text to an implementation-defined text representation of the selected options, and return."
I guess you're correct that it's not testable if it's conforming to show one of the selected option's text.
Checking that clicking an option in a customizable single-select changes select.value is probably good enough, but since it was questioned whether this is tested or not, I want to add detailed test coverage which proves that the other option is deselected. Context: whatwg/html#12069 (comment) Change-Id: Iab6b6f6e7dfc3fabe8623083531ee15f372681ae
Checking that clicking an option in a customizable single-select changes select.value is probably good enough, but since it was questioned whether this is tested or not, I want to add detailed test coverage which proves that the other option is deselected. Context: whatwg/html#12069 (comment) Change-Id: Iab6b6f6e7dfc3fabe8623083531ee15f372681ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7726423 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1609971}
Checking that clicking an option in a customizable single-select changes select.value is probably good enough, but since it was questioned whether this is tested or not, I want to add detailed test coverage which proves that the other option is deselected. Context: whatwg/html#12069 (comment) Change-Id: Iab6b6f6e7dfc3fabe8623083531ee15f372681ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7726423 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1609971}
Checking that clicking an option in a customizable single-select changes select.value is probably good enough, but since it was questioned whether this is tested or not, I want to add detailed test coverage which proves that the other option is deselected. Context: whatwg/html#12069 (comment) Change-Id: Iab6b6f6e7dfc3fabe8623083531ee15f372681ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7726423 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1609971}
|
How does this not have AAM implications? This also impacts the interaction model, no? |
|
I am updating a11y mappings for the prior listbox vs dropdown box change here: w3c/html-aria#555 Is that missing anything included in this PR? |
|
html-aria is for authoring requirements, html-aam is for UA requirements. I think this needs changes in https://w3c.github.io/html-aam/#el-select-listbox and the next section. |
…option elements, a=testonly Automatic update from web-platform-tests Add test coverage for deselecting other option elements Checking that clicking an option in a customizable single-select changes select.value is probably good enough, but since it was questioned whether this is tested or not, I want to add detailed test coverage which proves that the other option is deselected. Context: whatwg/html#12069 (comment) Change-Id: Iab6b6f6e7dfc3fabe8623083531ee15f372681ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7726423 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1609971} -- wpt-commits: ca9ae4817411eb7560f285baf783a82315137aba wpt-pr: 58981
|
Thanks, here is an html-aam PR: w3c/aria#2771 I put it in the OP |
|
All of the checkboxes in the OP are checked. Is there anything else I can do to get this merged? |
domfarolino
left a comment
There was a problem hiding this comment.
This LGTM with two small nits. Since this PR has been ready for a little while and has a completed OP and full browser support, I will land this sometime tomrrow.
| <li> | ||
| <p>If <var>select</var> has the <code data-x="attr-select-multiple">multiple</code> | ||
| attribute, then set <var>select</var>'s <span>select fallback button text</span> to an | ||
| implementation-defined text representation of the selected options, and return.</p> |
There was a problem hiding this comment.
I think we should only do this when the button element is not used as per the issue for customizable select v1.
There was a problem hiding this comment.
Do you mean that we should not set the fallback text in the case that the developer has provided their own <button> inside the <select>? I'm not sure it matters in that case, right, since this is <slot> fallback content and won't be displayed in that case anyway. Perhaps I'm misunderstanding the question though.
There was a problem hiding this comment.
I guess it depends on how we define this for the native appearance. My prototype puts the button element contents here and does not slot the button element in that case.
There was a problem hiding this comment.
Yeah I guess that maybe this step isn't great because the "select fallback button text" is only defined to exist as something that is used for base appearance. If I got rid of it, would that address your concern? If not, what exactly should we do instead?
If we only do this when the author doesn't provide a button element, that seems like something that would lead to invalidation issues later when adding or removing a button element. This content will be shown or hidden as expected based on the slotting of the author provided button element.
I guess it depends on how we define this for the native appearance. My prototype puts the button element contents here and does not slot the button element in that case.
For native appearance, we aren't actually defining the structure of the shadow root are we?
There was a problem hiding this comment.
No, but we should define the "button text" or "button".
There was a problem hiding this comment.
So we should either add a condition that the select fallback button text is not changed here if there is not an author provided button element, or add a definition for "button text" or "button" for appearance:auto? Adding the condition sounds like an easier change.
There was a problem hiding this comment.
I can see how defining the text used in the button for appearance:auto dropdown select is useful for #12474 but I'm still not sure that means we should change anything here.
This PR allows drop-down select elements with the multiple attribute to have base appearance. It is very similar to base appearance for single-select drop-down select elements.
I also added missing steps to deselect other selected options for single-selects when the user picks an option.
Fixes #12049
(See WHATWG Working Mode: Changes for more details.)
/form-elements.html ( diff )
/rendering.html ( diff )