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

[Popup] Should the delegatesfocus attribute be renamed? #368

Closed
melanierichards opened this issue Jul 1, 2021 · 28 comments
Closed

[Popup] Should the delegatesfocus attribute be renamed? #368

melanierichards opened this issue Jul 1, 2021 · 28 comments
Labels
popover The Popover API

Comments

@melanierichards
Copy link
Collaborator

melanierichards commented Jul 1, 2021

In PR #355, @mfreed7 mentioned on a discussion regarding the delegatesfocus attribute, which tells the user agent to move focus to the popup's first focusable descendent when the popup is shown:

If you're referring to this version of delegatesfocus, that's fairly different. That refers to Shadow DOM and how focus is proxied into the shadow root (and isn't well specified, at least in DOM/HTML specs). I bring this up for two reasons:

I do think this version here needs a specific definition like this, to be clear about what it does.
I am wondering if we should overload this attribute name for two similar but different concepts. Not sure if it helps or hurts developer understandability.

Opening this issue to generate discussion on whether delegatesfocus is the right name for this attribute.

Edit: when this to-be-renamed attribute is applied to popup, focus will be automatically sent to the first tab-focusable descendant of the popup, when that popup is shown. So for example:

<popup autofocus delegatesfocus>
  <p>I'm just some friendly text</p>
  <button>I will be focused, not the popup itself</button>
</popup>

The <button> would get focus when this <popup> is shown. Otherwise, focus would remain wherever it was when the popup was shown; alternatively, a dev could add the autofocus attribute to <popup>, and the <popup> itself would take focus.

Note: we are discussing on #367 how this attribute should be extended to other elements on the platform. There's sort of an open question as to whether:

  • This would be applicable to other elements that don't have a show() event, as popup and dialog do
  • If so, whether it would:
    • Automatically steal focus
    • Only send focus from an element to its descendant when the element itself is meant to receive focus (e.g. user tabs to element with delegatesfocus applied)
    • Have some option to specify either of the prior behaviors
    • Be composable with autofocus (i.e. delegatesfocus does not steal focus unless the element to which it's applied also has autofocus) <-- IMHO this could be a nice idea
@chrisdholt
Copy link
Collaborator

I don't know that I have a better name or semantic here, but I would agree on renaming - I saw this and got confused as to how this was related to delegating focus to a shadow root.

@gregwhitworth gregwhitworth added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jul 6, 2021
@bkardell
Copy link
Collaborator

catching up on details now, and I didn't realize at first this was not regular delegates focus because they do both ...delegate focus. I guess that means something different in both contexts that is unfortunately unclear... I suppose the native one kind of 'proxies' while this one kind of 'redirects' (to use webby terms I think we might have similar understanding of)?

@gregwhitworth
Copy link
Member

This was discussed in the telecon today and we resolved as follows:

Rename delegatesfocus content attribute. Name TBD

@melanierichards
Copy link
Collaborator Author

As part of the resolution, folks please feel free to chime in on this issue with a suggestion for an HTML content attribute with the behavior "send focus to my first tab-focusable descendent when I am shown".

@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jul 27, 2021
@melanierichards melanierichards added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jul 27, 2021
@melanierichards
Copy link
Collaborator Author

melanierichards commented Jul 29, 2021

Trying to generate some ideas for bikeshedding an attribute name for "send focus to my first tab-focusable descendent instead of myself"...

  • transfersfocus: does that seem like focus could be sent to a non-descendent?
  • bequeathsfocus: "bequeath" is the opposite of "inherit" but it seems a little fancy 🤵 for an attribute
  • focusdescendant / focusdescendants
  • passesfocus
  • These imply a particular behavior if the attribute is extended to non-event-driven elements, like a div instead of a popup or dialog (i.e. descendants would get focused immediately on "page load"); relevant to discussion on Should the delegatesfocus attribute be extended to other HTML elements? #367:
    • autofocuschildren
    • autofocusdescendants
    • autofocuscontent
  • passthroughfocus
  • passfocus

@bkardell
Copy link
Collaborator

bkardell commented Jul 29, 2021

I think I offered relaysfocus or redirectsfocus on the call as options that made sense to me - but transfersfocus isn't bad

@melanierichards
Copy link
Collaborator Author

Thanks @bkardell, forgot about those. relaysfocus seems short and sweet and clear.

@Wolfr
Copy link

Wolfr commented Jul 29, 2021

'passfocus' seems like a winner to me.

@maxsteenbergen
Copy link

How about entrustFocus?

@bkardell
Copy link
Collaborator

bkardell commented Jul 29, 2021

routesfocus came to me later, I guess familliar terms like route, redirect or relay seem likethings hopefully people can kind of relate to and might mentally conjure up the relationships - is my thinking

@hidde
Copy link
Contributor

hidde commented Jul 29, 2021

+1 to Melanie's transfersfocus and passesfocus, and Brian's suggestions of relaysfocus and redirectsfocus.

@css-meeting-bot
Copy link

The Open UI Community Group just discussed [Popup] Renaming delegatesfocus attribute.

The full IRC log of that discussion <gregwhitworth> Topic: [Popup] Renaming delegatesfocus attribute
<una> Github: https://github.com//issues/368
<gregwhitworth> melanierichards: I'm not sure if we're ready to resolve today
<gregwhitworth> melanierichards: last time around we resolved that we need to rename delegatefocus attribute due to the conflict with the shadowRoot property
<gregwhitworth> melanierichards: let's review the names and see if anything makes sense
<gregwhitworth> melanierichards: would love to get the opinions of folks especially non-native english speakers
<gregwhitworth> melanierichards: relays-focus, transfersfocus, routesfocus, passesfocus
<gregwhitworth> melanierichards: the call is maybe to up-vote things and next week we can poll
<gregwhitworth> una: a lot of these names indicate some transfer but the way that we use it. Is it a passive transfer or a capture
<gregwhitworth> una: if it's transferring focus and it's giving it to someone else if it's taking it for itself then it's capturing
<gregwhitworth> melanierichards: it would be the former
<gregwhitworth> melanierichards: if it was the latter it would be the autofocus attribute
<hdv> q+
<gregwhitworth> ack hdv
<gregwhitworth> hdv: what happens if there is no focusable child?
<gregwhitworth> melanierichards: that's a good question, the focus would just remain on the element itself
<gregwhitworth> melanierichards: we could look at dialogue today and see how that error solution is handled
<gregwhitworth> una: is it possibly to add any pseudocode into the issue because it will help me understand the verbiage
<gregwhitworth> flackr: should it get focus if it doesn't have a child with focus
<gregwhitworth> flackr: because the author of the element wouldn't be expected
<gregwhitworth> gregwhitworth: this is where you figure out the best fail state
<gregwhitworth> hdv: I think moving focus is what gets read out, I would expect it to move even if the child doesn't have a focusable element you would want to be able to have the popup read out rather than the button
<gregwhitworth> ACTION: melanierichards to determine the error state if no focusable child exists in a delegatesfocus labeled ancestor
<gregwhitworth> q?
<gregwhitworth> davatron5000: one silly question - you have a present tense of the verb. Is there precedence there?
<gregwhitworth> melanierichards: that's up for flexibility
<gregwhitworth> melanierichards: the DOM property is in that state
<gregwhitworth> melanierichards: my brain seems to think it makes sense either way
<gregwhitworth> melanierichards: there's probably not enough precedent to change it
<gregwhitworth> end topic

@diondiondion
Copy link

  * `autofocuschildren`
  * `autofocusdescendants`
  * `autofocuscontent`

I like these over the other suggestions. IMO the ideal "universal" API would be something like autofocus="within". But I guess re-using an existing attribute is off the table.

I was also thinking about why autofocus works so well as an attribute name: it treats "focus" as a verb. Most other suggestions combine it with another verb, but to me the added verbs don't add much meaning and all feel more or less interchangeable (might as well go fully generic: handlefocus or setfocus). In the end I'm always left wondering what will be focused.

So I'd keep looking more in the direction of something like focuscontent or focuswithin.

@bradkemper
Copy link

Focus-within

@hidde
Copy link
Contributor

hidde commented Jul 31, 2021

focus-within would be too confusing I think, because in CSS the pseudo class with that name reflects some sort of state (does an element within this element have focus or not), not an action, and it is true if any child has focus, while delegatesfocus would delegate to one specific child: the first focusable one.

@melanierichards
Copy link
Collaborator Author

@diondiondion said:

IMO the ideal "universal" API would be something like autofocus="within". But I guess re-using an existing attribute is off the table.

I want to call out that yours is an interesting idea, and maybe autofocus is a boolean that could've/should've been an enum. :) If it were just about the user agent handling the attribute value appropriately, there's probably some logic that could work there even if it's a little messy (treat autofocus like autofocus="self", recognize autofocus="within", throw away any other keywords). Probably where we'd be more likely to run into compat issues would be getting/setting the IDL attribute, and any scripts that assume a boolean value. So, I think we would need to introduce something different but just wanted to say thanks for exploring this concept!

@melanierichards
Copy link
Collaborator Author

melanierichards commented Aug 3, 2021

Putting this to a vote

Now that we have a good collection of options, we'd like to put this attribute to a vote. I've collected a couple options that have multiple people interested. Please use the emoji reaction function on this comment to vote on one of the following options:

👍 = relaysfocus (can change tense to relayfocus)
🎉 = redirectsfocus (or redirectfocus)
♥ = transfersfocus (or transferfocus)
🚀 = passesfocus (or passfocus)
👀 = focuscontent

If for some reason you run into issues using the reaction function to vote, or don't have a Github account, please reach out and we'll find a way to make sure your input is counted. e.g., I shared this on Twitter, feel free to express your opinion as a direct response to the tweet.

@BrandonLive
Copy link

BrandonLive commented Aug 4, 2021

As a data point, I came here and saw the options before I knew what the proposed attribute actually will do, and I would not have intuited that behavior from any of these options. The last one, “focuscontent” came the closest for me. The others seem like they’d be better fit for a scenario where you wanted to actually forward focus from one element to another (e.g. when clicking this button, I want to focus a particular text box).

It seems this attribute will enable a side effect whenever the popup is shown, which led me to think of somewhat verbose names like “focuscontentonshow”.

It also seems this could also be viewed as controlling two behaviors:

  • Taking focus when shown
  • Routing/forwarding focus to the first focusable descendant in the tab order

I wonder if there’s merit to teasing those apart?

The first of those almost seems like it could be a parameter on the show() method, rather than an attribute. But perhaps there are other ways these get shown? Or just reuse autofocus for that?

The ideas about extending the “autofocus” attribute with multiple possible values are compelling, it’s a shame that compatibility concerns apparently make that approach unviable.

One other idea: “autofocuscontent”?

@JamesCoyle
Copy link

I agree with @BrandonLive here. All but focuscontent sound like they transfer focus on a particular user action rather than automatically focusing on element.

I think autofocus="content" would be ideal here or autofocuscontent if there are issues adding functionality to the existing autofocus attribute.

@diondiondion
Copy link

So, I think we would need to introduce something different but just wanted to say thanks for exploring this concept!

Thanks for the response @melanierichards.

Since autofocus can't be changed, how about introducing a new enum attribute initialfocus instead of yet another boolean? E.g., initialfocus="self" could be equivalent to autofocus, initialfocus="within" (or similar) could be used for the new behaviour. This way at least the new attribute will be open to future extension.

@getflourish
Copy link

Throwing in the concept of responders from Apple UIKit (https://developer.apple.com/documentation/uikit/uiresponder)

For HTML, I’ve leaned on that concept, often calling a method "focusFirstResponder" when I open a dialog or similar things.

If re-using autofocus is not an option, I can imagine a more generic attribute such as focus with a clear value, such as:

  • focus="firstResponder"

Or in the context of HTML:

  • focus="content"

The question would be wether there are other use cases other than focussing the first responder? Depending on that, it could be boolean or rather have a set of options.

I could imagine focus="skip" or focus="off" as a more explicit way compared to specifying tabindex="-1".

@no-identd
Copy link

This whole discussion reminds me of the Microsoft Powershell approved verbs list.

@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Sep 27, 2021
@github-actions
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 Mar 27, 2022
@mfreed7
Copy link
Collaborator

mfreed7 commented Apr 8, 2022

Reading back through this issue, it sounds like focuscontent seems to be the winner. It got the most votes, and has a number of supporting comments.

Can we resolve to rename to focuscontent?

@mfreed7
Copy link
Collaborator

mfreed7 commented May 13, 2022

I'm going to Agenda+ this to discuss at a future meeting. There seem to be a number of options:

  • Go with focuscontent as it won the vote.
  • Add some late-breaking suggestions like autofocuscontent or focus=foo and vote again.
  • Remove delegatesfocus from the MVP for the Popup API, and keep discussing.

I'm beginning to be inclined to vote for the last option. Especially since we want to expand this attribute to other elements, there are issues with doing that, and the popup-specific use cases are nebulous at best. If a delegatesfocus-like attribute is added generally to the platform, it'll benefit Popups along with everything else.

So basically, Agenda+ to ask for a resolution removing delegatesfocus from the MVP for Popup.

@mfreed7 mfreed7 added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label May 13, 2022
@scottaohara
Copy link
Collaborator

the use case for delegatefocus is... if you have a popup where you know you need to set focus to something within it, but you don't know what the content of the popup will be, and hence you can't just use autofocus on the element which should receive focus?

@mfreed7
Copy link
Collaborator

mfreed7 commented May 14, 2022

the use case for delegatefocus is... if you have a popup where you know you need to set focus to something within it, but you don't know what the content of the popup will be, and hence you can't just use autofocus on the element which should receive focus?

Right, but I guess I'm wondering if that use case actually comes up, and in what context (and how often)?

@css-meeting-bot
Copy link

The Open UI Community Group just discussed [Popup] Should the delegatesfocus attribute be renamed?, and agreed to the following:

  • RESOLVED: The first version of the Popup API will not support the `delegatesfocus` attribute. We should open an issue to explore a more general `delegatesfocus` feature for all elements.
The full IRC log of that discussion <gregwhitworth> Topic: [Popup] Should the delegatesfocus attribute be renamed?
<gregwhitworth> github: https://github.com//issues/368
<JonathanNeal> q+
<JonathanNeal> ack JonathanNeal
<gregwhitworth> masonf: so the delegatefocus attribute was in the original explainer
<gregwhitworth> masonf: in the current prototype there are 2 attributes to set focus which is autofocus although it's re-interpreted when the element is shown it is focused
<gregwhitworth> masonf: the other is delegates focus and can only be placed on the element itself and find the first focusable element in the popup and set focus to it
<gregwhitworth> masonf: so it's 2 ways to achieve the same thing
<gregwhitworth> masonf: there is discussion to make this a generic feature for delegatesfocus
<gregwhitworth> masonf: there are a number of questions about that
<gregwhitworth> masonf: what's the usecase?
<gregwhitworth> masonf: the first proposal of delegatesfocus not support it
<gregwhitworth> masonf: we punt this to another discussion
<JonathanNeal> q+
<gregwhitworth> masonf: it may/may not be a generic solution
<gregwhitworth> JonathanNeal: I was going to ask if it's apt whether or not if it would reflect back into declarative shadow DOM which has shadowroot delegates focus, would this be similar and apply to both
<gregwhitworth> masonf: that's a good question and that is what spawned the bikeshed discussion
<gregwhitworth> masonf: that's the declarative solution for a shadowDOM host property that handles focus and it's not the same
<gregwhitworth> masonf: although they sound similar
<gregwhitworth> masonf: if we were to keep this feature in the first version then we would need to review and rename it
<gregwhitworth> ack JonathanNeal
<gregwhitworth> q?
<scotto_> +1 for punt. autofocus would largely achieve. and delegatefocus is really beneficial in a situation where someone doesn't know what content is goign to load within a popup (or other)
<dandclark> +1 to punting. Seems we don't have a strong use case since autofocus can achieve the same thing. And I'm concerned about confusion with the slightly different attachShadow delegatesfocus.
<JonathanNeal> +1 to punting, otherwise +1 to it being either a generic that plays well with Declarative ShadowDOM, or is well distinguished from Declarative ShadowDOM. (ref: `<template shadowroot="open" shadowrootdelegatesfocus>`)
<gregwhitworth> gregwhitworth: we'll need it for scenarios like when a popup appears such as logins
<gregwhitworth> masonf: you can do that today
<gregwhitworth> masonf: the scenario that this focuses is on is when you don't actually control the other components
<gregwhitworth> scotto_: we have some scenarios at Microsoft that could use this
<gregwhitworth> scotto_: but that speaks to your point that this has more uses as a generic attribute
<bkardell_> +1
<masonf> Proposed resolution: The first version of the Popup API will not support the `delegatesfocus` attribute. We should open an issue to explore a more general `delegatesfocus` feature for all elements.
<una> q+
<gregwhitworth> ack una
<una> q+
<masonf> RESOLVED: The first version of the Popup API will not support the `delegatesfocus` attribute. We should open an issue to explore a more general `delegatesfocus` feature for all elements.
<gregwhitworth> ACTION: masonf to open an issue for a general solution for delegatesfocus
<gregwhitworth> una: I notice in the explainer it still says that popup=popup, is there someone that will update that?
<gregwhitworth> masonf: I do plan to do that with all of the resolutions we've made; thanks for the reminder
<JonathanNeal> :) teamwork! :)

@mfreed7 mfreed7 closed this as completed May 26, 2022
@mfreed7 mfreed7 removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label May 26, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 4, 2022
The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 5, 2022
The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
aarongable pushed a commit to chromium/chromium that referenced this issue Aug 5, 2022
The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811398
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032067}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 5, 2022
The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811398
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032067}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 5, 2022
The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811398
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032067}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 5, 2022
…p, a=testonly

Automatic update from web-platform-tests
Remove delegatesfocus behavior from popup

The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811398
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032067}

--

wpt-commits: 3b3bc3508d17e0fbfe335a4fd5c593a3c0a4fdb6
wpt-pr: 35345
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811398
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032067}
NOKEYCHECK=True
GitOrigin-RevId: 68e42f3582d3b1d63d712b5f1ed080fa0f01f6ff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API
Projects
None yet
Development

No branches or pull requests