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

Custom actions / variety of actions for invoker leads to ambiguity for accessibility #898

Open
scottaohara opened this issue Oct 18, 2023 · 22 comments

Comments

@scottaohara
Copy link
Collaborator

scottaohara commented Oct 18, 2023

I'll start right off and say this is a major misunderstanding on my part from when i first reviewed the explainer - as I assumed the primary intent was for replacing popovertarget with a more generic attribute that would allow for the showing/hiding of other elements on the page, per my toggle attribute/button issue #700

But it wasn't until talking about this briefly with @keithamus last week that I finally realized this is trying to do a lot more than just showing/hiding content. And looking at this again now, there is quite a bit more that needs to be considered for the accessibility section - as that doesn't account for many use cases - which is 'fine'... but I'm currently wondering if it can be assumed that like popovertarget, an invokertarget can simply default to exposing an expanded/collapsed state by default.

The listing of action hints have a variety of other properties that might need to be exposed in addition, if not in place of the expanded/collapsed state. Some of them might not need a state conveyed at all - like a regular button - but instead would benefit from a confirmation of action notification (e.g., clearing a canvas - if an announcement of 'cleared' was made, that'd inform the user that the button did something... but alas the notificatin api is not yet a thing).

Per this bit of the explainer:

This allows for custom actions. Built-in interactive elements have built-in behaviours (detailed below) which are determined by the invokeaction but also Invokees will dispatch events when Invoked, allowing custom code to take control of invocations without having to manually wire up DOM nodes for the variety of invocation patterns.

This says to me that there needs to be a super big "if you create your own actions, you're in charge of all the accessibility properties" - which may be fine? But it's going to be interesting to figure out how to slice up all these default actions, get the right mappings for their common use cases, and then also provide instructions for what to do in cases that deviate from common usage.

@keithamus
Copy link
Collaborator

assumed the primary intent was for replacing popovertarget with a more generic attribute that would allow for the showing/hiding of other elements on the page

This is part of the intent but I think the desired scope has creeped quite a bit. In general I'd like to think of invoketarget as a way of interacting with any interactive element on the page. Today that is mostly showing/hiding or expanding/contracting, but it also is controlling elements like <video>/<audio>, and so I agree there's a definite ambiguity from the attribute of the button alone; it needs the context of the target element to exist on the page to make a determination about what aria mappings to apply.

as that doesn't account for many use cases - which is 'fine'... but I'm currently wondering if it can be assumed that like popovertarget, an invokertarget can simply default to exposing an expanded/collapsed state by default.

Yes I agree, the accessibility section probably needs a table similar to the one describing the actions; or perhaps it can be added as column against that.

The listing of action hints have a variety of other properties that might need to be exposed in addition, if not in place of the expanded/collapsed state. Some of them might not need a state conveyed at all - like a regular button - but instead would benefit from a confirmation of action notification (e.g., clearing a canvas - if an announcement of 'cleared' was made, that'd inform the user that the button did something... but alas the notificatin api is not yet a thing).

I'm very happy to get rid of the clear canvas part of this explainer; I didn't consider it as something which would make it to the concrete spec/implementation, but more of a canary for more discussion around the scope & possibilities of the button/target combo.

Per this bit of the explainer:

This allows for custom actions. Built-in interactive elements have built-in behaviours (detailed below) which are determined by the invokeaction but also Invokees will dispatch events when Invoked, allowing custom code to take control of invocations without having to manually wire up DOM nodes for the variety of invocation patterns.

This says to me that there needs to be a super big "if you create your own actions, you're in charge of all the accessibility properties" - which may be fine?

I think this is an acceptable trade-off; custom interactions already are heading off the beaten-path as they'll be reacting to events (which is the opposite of what the internal code does).

There's maybe something we could do for exposing an API where the button can query its invoketarget to figure out available actions and what they do, to get enough information to be able to establish some aria mappings but that is perhaps an iteration or two beyond the MVP?

But it's going to be interesting to figure out how to slice up all these default actions, get the right mappings for their common use cases, and then also provide instructions for what to do in cases that deviate from common usage.

I'll hopefully get a more concrete set of mappings specified as we iterate on the implementations. Between now and release of this feature I anticipate a lot of changes in this area anyway.

@lukewarlow
Copy link
Collaborator

Would the mappings for some of these default actions be available in the browser's already. For example a button that triggers an element to go full screen already exists inside of video elements?

I'm assuming showPicker is a simple aria-expanded but wouldn't necessarily have a native counterpart right now?

@keithamus
Copy link
Collaborator

Would the mappings for some of these default actions be available in the browser's already. For example a button that triggers an element to go full screen already exists inside of video elements?

I think the problem is there is an ambiguity, consider the following examples, and how they might map:

<button invoketarget="doesntexist">Open</button>
<button invoketarget="invalid" invokeaction="showPicker">Open</button>

<div id="invalid"></div>

Existing buttons within the browser are created with an explicit singular purpose and so have a singular mapping. Buttons with invoketarget could be invalid either by pointing to a non-existent element, or a valid element but with an "invalid" action.

@lukewarlow
Copy link
Collaborator

My assumption for both of those would be 0 accessibility mappings. The second one is effectively a custom action (see #900 about that).

To me I would guess it's the dynamic changing of the aria that's the difficulty mapping the default actions and elements to state should be relatively simple? (Probably a lot of permutations though)

@keithamus
Copy link
Collaborator

My assumption for both of those would be 0 accessibility mappings.

I share the assumption, but that me be problematic, and that deviates from popovertarget which Scott has proposed to set aria-details and aria-expanded. My understanding of that spec, and a discussion with Scott, is that a button with a popovertarget will set aria-expanded=false if the popover doesn't exist on the page (@scottaohara please correct me if I mustunderstood).

It might be problematic because if an AT device lands on a <button invoketarget=something> where the something isn't on the page, what information should it convey? It's a common idiom within React to not render an element unless it's visible, and so a React app may include buttons to dialogs where there is no dialog on the page yet. One could argue that for those cases there is an expectation that the developer should add those attributes themselves, but it somewhat belies the purpose of invoketarget - which aims to alleviate the burden of assigning the right aria attributes for you (the same is also true for custom actions but there's that's an even more difficult problem to solve).

@lukewarlow
Copy link
Collaborator

But an idref wouldn't work for an element that's not on the page (read this as in the DOM)? So that react use case wouldn't work? In that case you'd still need to use a click handler to add the dialog to the DOM at which point you might as well call showModal() too?

@keithamus
Copy link
Collaborator

For the custom case, maybe a wild idea, but invoketarget could call an API on the element which custom elements could then override (bikeshedding very much welcome):

interface InvokeTargetAriaMappingsReturn {};
InvokeTargetAriaMappingsReturn includes ARIAMixin;

partial interface HTMLElement {
  invokeTargetAriaMappings(DOMString action): InvokeTargetAriaMappingsReturn?
}

When an invoketarget is parsed/assigned, it calls the targets .invokeTargetAriaMappings(invoketargetaction) and assigns the button the attributes that are returned.

This might also help with Feature Detection (see https://github.com/keithamus/invoker-buttons-proposal/issues/24); one could call invokeTargetAriaMappings('new-thing') and if null is returned then the feature likely doesn't exist.

@lukewarlow
Copy link
Collaborator

lukewarlow commented Oct 21, 2023

For the custom case, maybe a wild idea, but invoketarget could call an API on the element which custom elements could then override (bikeshedding very much welcome)

Could we make a new event invokerattached that fires on the target. (Perhaps would need be invokeactionchanged or something aswell/instead of)

And then you can just use the AOM aria reflection APIs?

That wouldn't address feature detection but does limit any new APIs to just events.

@scottaohara
Copy link
Collaborator Author

if invokeaction was always required, then at least that would give a good indication of what the button was meant to control, and even for the cases where the element doesn't yet exist in the DOM, then some baseline of a11y mappings could still be provided.

if invoketarget can be set without an invoketarget action, then would it even do anything? would it just default to a show/hide of the referenced iD? it just seems being as explicit as possible would mitigate some of the ambiguity. The tradeoff there being that authors need to always declare the 2 attributes - but also, isn't that far better than having to write the JS and handle all the necessary ARIA attributes one self? I think that's probably a tradeoff worth dealing with.

If an invoketarget points to an element where the invokeaction cannot fire, then is that just an author error and the whole thing breaks anyway? if something breaks, then devs are more likely to fix it, rather than let it slide and have the browser have to guess at what they meant to do (and have the potential to get it wrong).

@keithamus
Copy link
Collaborator

keithamus commented Oct 24, 2023

if invoketarget can be set without an invoketarget action, then would it even do anything?

An invoketarget with no invokeaction set would be an implicit invokeaction=auto, which does things in most cases; for popover it's the alias of togglePopover(), for <dialog> it's "togglemodal" (an invented concept for invokers). I don't think there is any ambiguity in the mapping provided the element exists in the DOM as it is declared.

If an invoketarget points to an element where the invokeaction cannot fire

I don't believe that's possible, unless it points to an ID that is not on the page. If there is no available default action, the event will still fire. For example:

<button invoketarget=d invokeaction=tooglemoodal>Intentional Typo</button>
<dialog id=d></dialog>

This won't open the dialog, but an new InvokeEvent('invoke', {action:'tooglemoodal', invoker}) will be fired on the <dialog>.

@lukewarlow
Copy link
Collaborator

For the custom case auto would need to be handled in JS, but then so would an explicit action. I don't think requiring an action be defined alleviates any issues with ambiguity? You'd need to handle the aria stuff manually regardless (for the custom case)?

And for built ins there's not really any ambiguity?

@keithamus
Copy link
Collaborator

It might help to read the draft spec for popover (whatwg/html#9875, the relevant section: https://whatpr.org/html/9875/invokers.html#invoke-target-attribute-activation-behavior), to get a sense of what ambiguities lie (which is to say, I don't think it's that ambiguous). There are undefined cases where invokeaction has been explicitly set to something that isn't built-in, but I think it's okay to give up in those cases because they're likely custom behaviours.

@scottaohara
Copy link
Collaborator Author

i dunno. to me, auto makes sense in the scope of popover where there was one consistent behavior (showing/hiding). here that's not true with various other types of invoke actions being thrown into the mix.

the ambiguity does remain if auto can change based on the invoked element. e.g., invoking a popover or invoking a dialog in the modal state. (i say this because i'm also not 100% convinced there isn't still a use case for dialog.show(). there are some 'canvas' based apps (re: not <canvas>) where persistent dialogs need to be present, but should be allowed to be scroll out of view / not appear atop other content

@lukewarlow
Copy link
Collaborator

This wont impact on the auto for built in elements but #900 might be worth thinking about alongside this issue too.

@lukewarlow
Copy link
Collaborator

Does requiring an explicit action actually help fix the ambiguity? Aren't we always going to rely on the invokee element in some form? Such as for the toggle action? If we always need to look up the invokee element doesn't that negate any benefit to removing the auto state?

@scottaohara
Copy link
Collaborator Author

scottaohara commented Oct 24, 2023

to be honest, i'm not sure... as i mentioned, i think there are things about this proposal that i'm just not getting, and what to do as default mappings for this seems strange if the default can change based on what the target is.

but circling back to what i do think i understand, is that

  • the ability to show/hide static content that is not a details element is not possible unless toggle could be expanded to cover other elements.
  • if the default target exists in the DOM, the mappings would be based on that target element, and that's where potential mixing of things seems unclear to me. e.g., <video popover> what does auto do here? play, show the popover, both? (what a yucky example). But otherwise, auto doesn't really mean anything and mappings need to be determined based on the referenced element / modified based on the invokeaction. e.g., a button targeting an audio/video with a 'mute', play or pause action could maybe get an aria-pressed by default - but playpause or auto would get nothing since we have no idea if they're toggling the visible label of the control or not.
  • if the action is custom/mistyped, then we have no idea what the dev wants to do, so then it's still all on them to get that right.
  • if an action is called on an element that doesn't support that action in its current state (e.g., input type=radio showPicker or select size=5 showPicker) then i would assume do nothing?

hmm. i guess another way to put this is that it can be defined exactly what to do, or not, for each potential invokee based on specific actions, but for the play/pause example for audio/video elements, we have no idea if the developer is going to keep a consistent accessible name for that control, or change it based on state. And if its a consistent name, then we would use an aria-pressed mapping - but if the developer is going to change the accname/visible identifier for the control when it toggles, then we do not want to also map the pressed state change. Similar for any sort of toggling with details. If someone changes the name (expand/read more to collapse/show less, for example) again we don't want an aria-expanded state on those as well.

i dunno. hopefully this helps relay where my head is stuck.

edit: actually, wait. how does the play/pause/mute account for the fact that those are often coupled with a visual change (icon or text)? Wouldn't this still require someone use JS anyway? or would it be expected to change this via CSS, which would then need a state on the button(?) to hook into?

@keithamus
Copy link
Collaborator

keithamus commented Oct 24, 2023

if the default target exists in the DOM, the mappings would be based on that target element, and that's where potential mixing of things seems unclear to me. e.g.,

popover always takes precedence in the order of operations. So auto on any element with popover will toggle the popover. This also means <dialog popover> will trigger the popover not the dialog behaviour.

if an action is called on an element that doesn't support that action in its current state (e.g., input type=radio showPicker or select size=5 showPicker) then i would assume do nothing?

Yes this is correct. We can make a console.warn appear here to guide the developer on the right path.

actually, wait. how does the play/pause/mute account for the fact that those are often coupled with a visual change (icon or text)?

Right now this isn't accounted for in the proposal. I think that's okay? Perhaps this establishes a use case for a <button type=toggle> 😜?

@lukewarlow
Copy link
Collaborator

lukewarlow commented Oct 29, 2023

I feel like there must be some CSS selector that will allow styling an invoker button based on the targets state. But I can't think of a foolproof way off the top of my head.

I would then say maybe we should introduce a ::invoketarget to style an invoker based on it's invoketarget state pseudos' But something like input:has(::file-selector-button:hover) doesn't actually seem to work as I might expect it too.

Perhaps there really is no way to do this currently?

It seems we'd need to define it as a has allowed pseudo element if we wanted this to work.

Screenshot_20231029-014157.png

@lukewarlow
Copy link
Collaborator

lukewarlow commented Nov 1, 2023

Disclaimer I haven't given OpenUI's tab research a read through yet so apologies if this is crazy talk. I will give it a read tomorrow these are just some late night thoughts.

https://jsfiddle.net/qv2b5o03/ - is my attempt at building something resembling an accessible tab system making use of exclusive accordions and invokers. I have probably committed many cardinal sins for which I apologise. (it will also only work on Chrome Canary with experimental flag).

It led me to thinking could some of this aria stuff be automatically handled?

  • I assume invoke buttons will get aria-controls automatically?
  • Is the aria-selected handling possible, can we map it based on both the target and invoker elements actual aria roles rather than element semantics?
  • details is supposed to not support any aria roles but at least in my testing it works as a tab panel just fine (tested with VoiceOver), what gives? Is MDN wrong?

@lukewarlow
Copy link
Collaborator

Linking #622 to continue the styling discussion

@lukewarlow
Copy link
Collaborator

lukewarlow commented Nov 18, 2023

Right now this isn't accounted for in the proposal. I think that's okay? Perhaps this establishes a use case for a <button type=toggle> 😜?

See #957

Copy link

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 May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants