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

Update HTML parser changes in select explainer #1077

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

josepharhar
Copy link
Collaborator

Based on discussion and developer feedback in the WHATWG issue about HTML parser changes for , we are not requiring to be added to the author HTML for most cases. This PR updates the explainer to remove in a bunch of places. https://github.com/whatwg/html/issues/10310

Based on discussion and developer feedback in the WHATWG issue about
HTML parser changes for <select>, we are not requiring <datalist> to be
added to the author HTML for most cases. This PR updates the explainer
to remove <datalist> in a bunch of places.

whatwg/html#10310
Copy link
Collaborator

@mfreed7 mfreed7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with just a few things

site/src/pages/components/customizableselect.mdx Outdated Show resolved Hide resolved
@@ -177,14 +170,12 @@ This example uses the customizable `<select>` element with custom CSS to target
<button>
<selectedoption></selectedoption>
</button>
<datalist>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the <datalist> still allowed? If so, want to keep one example with it? Somewhere, not necessarily this one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's still allowed, and supporting it imposes quite a lot of complexity. I think I'm going to have to work some more on justifying it.

So far I can think of 3 reasons:

  1. If we reinstate the <input> creating a </select>, then <datalist> would be needed in order to support <input>. Unfortunately <input> is not part of the content model and something which we can make accessible yet, so this isn't a great argument.
  2. In order to make a pseudo element which targets the shadowroot <datalist> as capable as the element itself, we will probably have to do a bunch of work in CSS in order to make sure that all pseudo-elements can be used on it among other things.
  3. View transitions might need an actual javascript handle to the element, but I'm not sure. There could also be other cases, like if the author wanted to show or hide the popover from script for some reason.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm following up with adam to see if we can get a view transitions demo which I could put in the explainer, which would require and justify <datalist>

</option>
</datalist>
<option>
❤️ <span class=description>heart</span>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can't imagine the intent of these options is to have them announced as "red heart heart" or "slightly smiling face smile"

if we're going to demonstrate emoji paired with text, we should probably mark the emoji as aria-hidden=true. might be worth having a note after this to indicate the why as well (to reduce awkward redundancy, to reduce the exposed / expected accName that someone would need to figure out if using some voice control software, etc.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I wrapped them in <span aria-hidden=true>, but that also means that the aria-hidden is copied into the <selectedoption>. How do you think the button should be announced?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you're pairing it with text, then it's common practice/expected that the visible text is what is announced.

assuming that the emoji (aria-hidden) and the text are copied to the selectedoption - then that is in line with what i'd expect to happen. if there's somehow something special about the span class=description and that not being copied over / either due to implementation or because of what the author wants to do here, then that's another reason we probably need <description> instead of faking it by using a meaningless span / class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new PR to work on this example: #1079

@@ -286,11 +275,9 @@ Here is another example with custom content in the listbox: [codepen](https://co
<button><selectedoption id=myselectedoption></selectedoption></button>
<select selectedoptionelement=myselectedoption>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know everyone really wants to use this for split buttons, but we should come to consensus on what a good markup pattern would be for that, and if there are any additional accessibility considerations we should make before we actively promote it as an example. Or, in lieu of having a more robust pattern - we should explicitly note that this is merely a simplified version of only the start of what someone would need to do to create a split button, and this markup pattern is not a recommendation in and of itself.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the idea of saying that this new attribute we're adding to the platform is so hard to use that we can't show you how to use it correctly.

The text inside the selectedoption inside the button accurately describes what action the button will do when activated. What would be better?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, fortunately i'm not saying it's going to be too hard to use. but i am saying we haven't really discussed what we should promote as a markup pattern (or patterns) for this - so we should do that.

for instance, it seems you're assuming i have a problem with the button the selectedoption is placed within - but that's not my concern. Rather, what is one to expect the select's accessible name should be per the pattern you wrote? Nothing? Down arrow? neither of those are particularly good answers.

so how do we want to recommend people label the select? or should we consider ways to automatically name the select / associate it somehow with the selectedoption still?

also, commonly component libraries have coded the 'more options' portion of the split button (the select) as a button that invokes a role=menu popup. My assumption with what we were doing here is simply giving people a way to more easily markup their custom split buttons, and then it would be on authors to finish the rest of the setup on their own. but am i wrong in assuming we were on the same page? is there appetite in implementing more of this for authors automatically - to the point that your simple markup pattern would do more by default, and thus would then be sufficient?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another question. would the button part be necessary for this sort of pattern, or if one were to leave it out, would the existence of the selectedoptionelement attribute on the select be enough for the browser to know this is a customized select, and render a UA default down chevron for the select, without someone having to add in their own custom button part/icon?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while since I've looked at this, but I'm going to jump in since I've done some work on both splitbuttons and custom combos & their accessibility.

I agree with @scottaohara, this pattern has a number of accessibility implications that are not addressed or handled explicitly. Splitbuttons have a history as a Windows native control that combines a primary action with a menu, but does not easily map to the web for reasons including the following:

  • Having two actions for one control causes problems for different access modalities, especially outside of a desktop environment (e.g. voice and touch screen readers)
  • Splitting it out into two separate control types helps with this, but raises questions around tab/arrow focus targets and behavior as well as naming (as @scottaohara already brought up).
  • Roles are a little difficult. There's a control type for this in Windows, but not in other platforms, at least not that I'm aware of. Splitting this into two controls means it's even more unclear how you would create markup that shows the two buttons(?) are related.

Additionally, splitbuttons generally open a menu, not a listbox, and a <select> is not a good fit for the menu portion. Since <select> is a form field, it has certain screen reader navigation & shortcut behaviors that do not make sense for the menubutton portion of a splitbutton. It also exposes a value, and it's not clear how that would be presented to the user in an understandable way when that value is presented to the user in the primary action button but not in the combobox/menu button.

I get the motivation behind making a splitbutton, but I think any effort to do so would be better served by being independent of <select> work. Trying to duct tape them together seems like it would degrade the accessibility and usability of both the <select> and the desired splitbutton pattern.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also stepping back a bit from the splitbutton question -- I think I might be misunderstanding this because I've been out of the loop, but is selectedoption essentially just a way for a developer to drop the text of a <select>'s selected value anywhere on the page, or is it more constrained?

The idea of having an HTML mechanism to reflect the current value of a form control seems like quite a cool idea, would it make sense to have that mechanism be more general than just <select>? I could see essentially the same use case for being able to reflect the value of radios, sliders, general text-like inputs, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitbuttons are a use case we were trying to support, first by allowing multiple buttons to be slotted into the select, and now by having the selectedoptionelement attribute which allows the <selectedoption> to live outside the select.

It sounds like this case cannot be made accessible, so I will remove it from the explainer. This will also make it harder to ship the selectedoptionelement attribute, but hopefully we can make compliant examples.

I will remove the split button and selectedoptionelement example from the explainer: #1080

@josepharhar josepharhar merged commit f2809b2 into openui:main Aug 6, 2024
5 checks passed
@josepharhar josepharhar deleted the parser branch August 6, 2024 17:29
josepharhar added a commit to josepharhar/open-ui that referenced this pull request Aug 6, 2024
Split buttons cannot be made accessible:
openui#1077 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants