openui / open-ui

Maintain an open standard for UI and promote its adherence and adoption.
https://open-ui.org
Other
3.37k stars 182 forks source link

Imperative invoker relationships #1024

Closed mfreed7 closed 2 months ago

mfreed7 commented 2 months ago

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.
lukewarlow commented 2 months ago

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 commented 2 months ago
  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 commented 2 months ago

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

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 commented 2 months ago

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!

lukewarlow commented 1 month ago

@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 commented 1 month ago

@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.