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

Imperative invoker relationships #1024

Closed
mfreed7 opened this issue Mar 31, 2024 · 6 comments
Closed

Imperative invoker relationships #1024

mfreed7 opened this issue Mar 31, 2024 · 6 comments
Labels
agenda+ Use this label if you'd like the topic to be added to the meeting agenda invokers popover The Popover API

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 31, 2024

This is two issues in one:

  1. Should it be possible to do popover.showPopover({invoker: anotherElement}) and have that set the invoker relationship? Reminder: an invoker relationship allows popover to be "nested" inside anotherElement's containing popover, and it also fixes up the keyboard tab order so popover comes just after anotherElement. Both of these happen automatically for <button popovertarget=popover>.
  2. If the answer to the above is "yes", then what should be done about the invokers proposal? First, it doesn't connect popovers in the same way that popovertarget does (nesting plus keyboard behavior). Second, it doesn't have a proposed imperative API at all, that I know of.
@mfreed7 mfreed7 added popover The Popover API invokers agenda+ Use this label if you'd like the topic to be added to the meeting agenda labels Mar 31, 2024
@lukewarlow
Copy link
Collaborator

First, it doesn't connect popovers in the same way that popovertarget does (nesting plus keyboard behavior).

I don't fully understand what you're referring to but just wanted to say anything popovertarget does invoketarget should absolutely be doing and it's a spec bug if there's something missing.

@keithamus
Copy link
Collaborator

  1. Passing the invoker is useful, so yes I think it should be possible.
  2. a. As Luke says, if we're aiming to deprecate popovertarget then invoketarget should do the same thing. Arguably it makes sense for invoketarget to fixup tab order in all scenarios and not just for popovers (for dialog it's irrelevant but details/video/input etc?).
    b. I think the imperative API remains as showPopover. We could also extend showModal() to take {invoker}... TAG did mention a desire to add invoke(action, invoker) so maybe that should be the direction of travel?

@css-meeting-bot
Copy link

The Open UI Community Group just discussed Imperative invoker relationships, and agreed to the following:

  • RESOLVED: add an option to showPopover() to declare the invoking element.
  • RESOLVED: invoketarget=popover should create the same relationships (e.g. nesting and keyboard behavior) that popovertarget=popover does. Behavior TBD for interesttarget attribute.
The full IRC log of that discussion <hdv> masonf: I raised this one recently
<hdv> masonf: they're kind of two interrelated issues, I packed them into one
<hdv> masonf: when you use popovertarget in a button, various things happen for you
<hdv> masonf: more than I even mentioned in the issue
<hdv> masonf: eg for nesting popovers more easily
<hdv> masonf: another is that keyboard navigation is fixed up, so hitting tab takes you to the popover when it is open after you've opened it. That doesn't happen when you open it through script
<hdv> q+
<masonf> hdv: +1 basically, that makes sense. Let's do it.
<masonf> q?
<flackr> +1
<luke> +1
<luke> q+
<hdv> ack me
<masonf> ack luke
<hdv> luke: I think it makes sense… I do wonder if this should apply to other things too as Keith mentions? like <dialog>?
<keithamus> q+
<masonf> ack keith
<hdv> masonf: what's unique about popover is that a connection is formed in a very specific way, I don't think that's the case for dialog, they're more independent of their invoker
<hdv> keithamus: for a dialog that is modal we don't need to have that connection
<hdv> keithamus: another one could be <details>, maybe a similar relationship
<hdv> keithamus: not sure if good for stuff like <video>
<luke> q+
<hdv> keithamus: I feel anything but popover would be beyond v1 of invokers
<masonf> ack luke
<hdv> luke: I agree … only other thing would be input elements… I guess especially selects… effectively a popover but not exposed as a popover
<masonf> q+
<hdv> luke: but could come up with that in the future
<hdv> masonf: part two is should hte same apply to invokers?
<scotto> q+
<masonf> ack mason
<hdv> luke: if there's a difference between popovertarget and invokertarget it should be a deliberate choice
<brecht_dr> q+
<hdv> keithamus: you might be referring to case where there is no point for the aria-details mapping?
<hdv> [ relevant post that Scott and I wrote: https://hidde.blog/popover-accessibility/ ]
<hdv> masonf: so is it safe to assume they're the same?
<hdv> luke: seems to me yes?
<hdv> luke: another thing I wanted to ask would be is all of this different if a popover is invoked by hover?
<masonf> ack scott
<brecht_dr> q-
<hdv> scotto: for invoketarget it would matter what the invoked element was… there is no reason to give a details relationship to a modal dialog as nobody would ever experience that relationship as no user could ever experience it as it was on a button that isn't in the modal and the modal is the only thing that's interactable at that time
<hdv> scotto: for video… probably don't need to have the relationship on video, play is more of a state
<masonf> Proposed resolution: add an option to showPopover() to declare the invoking element.
<masonf> Proposed resolution: invoketarget=popover should create the same relationships (e.g. nesting and keyboard behavior) that popovertarget=popover does. Behavior TBD for interesttarget attribute.
<brecht_dr> +1
<keithamus> +1
<luke> +1 to both of those
<hdv> +1 to all of the above
<hdv> keithamus: we could look at adding this before the toggleevent
<hdv> keithamus: I think there was already a PR for that in the spec?
<hdv> masonf: there are two things I heard about beforetoggle… one was about what was the invoker, the other what was the trigger
<hdv> s/things/requests
<hdv> s/heard/heard at WHATWG
<masonf> RESOLVED: add an option to showPopover() to declare the invoking element.
<masonf> RESOLVED: invoketarget=popover should create the same relationships (e.g. nesting and keyboard behavior) that popovertarget=popover does. Behavior TBD for interesttarget attribute.

@tabatkins
Copy link

Oooh, thanks y'all.

For context: I raised this with Mason when exploring switching Bikeshed's info panels (showing information on dfn and a elements) to using popovers, so I could stop using the roving tabindex hacks to get good tabbing behavior. Unfortunately, you currently only get that focus behavior from a <button popovertarget>, which meant I was SOL.

Being able to specify the invoker in the JS call will solve my problems, thanks!

@mfreed7 mfreed7 closed this as completed Apr 9, 2024
@lukewarlow
Copy link
Collaborator

@mfreed7 would popover.showPopover({invoker: anotherElement}) work if the popover is in one shadow root, and anotherElement is in another? That would solve part of the issues with popoverTargetElement not working that way.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented May 13, 2024

@mfreed7 would popover.showPopover({invoker: anotherElement}) work if the popover is in one shadow root, and anotherElement is in another? That would solve part of the issues with popoverTargetElement not working that way.

It certainly could, assuming we did not create an API to retrieve the invoker. Otherwise, we risk opening access to a shadow root via this mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda+ Use this label if you'd like the topic to be added to the meeting agenda invokers popover The Popover API
Projects
None yet
Development

No branches or pull requests

5 participants