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

Focus delegate algorithm doesn't consider elements assigned to slot #9245

Open
sefeng211 opened this issue May 2, 2023 · 18 comments
Open

Focus delegate algorithm doesn't consider elements assigned to slot #9245

sefeng211 opened this issue May 2, 2023 · 18 comments
Labels
topic: focus topic: shadow Relates to shadow trees (as defined in DOM)

Comments

@sefeng211
Copy link
Contributor

Is this expected?

Consider this test case

<section>
    <input type="text" />
</section>
<button>
  Open slotted dialog
</button>

<script>
document.querySelector('button').addEventListener('click', () => {
  document.querySelector('section').shadowRoot.querySelector('dialog').showModal();
});

document.querySelector('section').attachShadow({mode: 'open'});
document.querySelector('section').shadowRoot.innerHTML = '<dialog><slot></slot></dialog>';
</script>

The input element will not be focused upon clicking the button according to the current spec.

@annevk annevk added the topic: shadow Relates to shadow trees (as defined in DOM) label May 5, 2023
@annevk
Copy link
Member

annevk commented May 5, 2023

cc @rakina @domenic

@domenic
Copy link
Member

domenic commented May 8, 2023

Is this a duplicate of #7207?

@domenic
Copy link
Member

domenic commented May 8, 2023

In general if you look through https://github.com/whatwg/html/issues?q=%22focus+delegate%22 it looks like we've definitely done this intentionally.

On the other hand, maybe at the time we did this, we were thinking mostly of delegatesFocus, and not about <dialog>. Maybe using a single "focus delegate" algorithm for both cases was too ambitious?

@Westbrook
Copy link

Browser support for this is pretty important to development at Adobe. Our Spectrum Web Components library (leveraged in Photoshop, Express, soon to be Illustrator, and many more major efforts at the company) are very interested in moving towards leveraging <dialog> but this missing feature requires a good deal of timing orchestration and possibly expensive/encapsulation breaking client side DOM traversal to ensure. What's worse, while it works out of the box in Chormium, but not in WebKit/Gecko make the management of that nuance even more precarious.

We'd really appreciate this becoming part of the specification in this area. 🙏🏼 🙇🏼

@jpzwarte
Copy link

We have the same use case within our Sanoma Learning Design System. We have an <sl-dialog> web component where all the content is slotted into a <dialog> element.

@sefeng211
Copy link
Contributor Author

Yeah, I think it might makes sense to have a different behaviour for <dialog>. The argument from Emilio made in #7207 still makes sense for Shadow DOM alone, which we consider the Shadow DOM tree separately from the light DOM, so it makes sense to delegate elements solely inside the Shadow DOM.

However for this use case, the container is not Shadow DOM, but rather <dialog> which is a light DOM element, so we should be able to delegate the focus to other light DOM elements.

cc @emilio in case Emilio has different opinions.

@josepharhar
Copy link
Contributor

Browser support for this is pretty important to development at Adobe. Our Spectrum Web Components library (leveraged in Photoshop, Express, soon to be Illustrator, and many more major efforts at the company) are very interested in moving towards leveraging <dialog> but this missing feature requires a good deal of timing orchestration and possibly expensive/encapsulation breaking client side DOM traversal to ensure. What's worse, while it works out of the box in Chormium, but not in WebKit/Gecko make the management of that nuance even more precarious.

@Westbrook are you relying on the slot behavior in stable chrome? I am trying to remove it but running into issues in chrome:// pages which also rely on it

@Westbrook
Copy link

@Westbrook are you relying on the slot behavior in stable chrome? I am trying to remove it but running into issues in chrome:// pages which also rely on it

@josepharhar we can’t rely on it due to me not finding a quality test to show it is there, user agent not withstanding. Due to this, we’ve JS’ed around it, but it should work this way in all browsers. Yes, the flat tree can be annoying. Yes, there a many APIs new and old that avoid it and other forms of support for shadow DOM realities. But, shadow DOM is a part of the web platform and it needs to be respected as the number of people who build with it is growing and will continue to making it more and more important to do so.

@josepharhar
Copy link
Contributor

A crbug about this has popped up: https://bugs.chromium.org/p/chromium/issues/detail?id=1459293
Someone wants to have slotted buttons be able to get initial focus in popovers

@smaug---- smaug---- added the agenda+ To be discussed at a triage meeting label Mar 25, 2024
@past past removed the agenda+ To be discussed at a triage meeting label Mar 28, 2024
@smaug----
Copy link

An idea (which I haven't thought through at all): slot element could have a flag/attribute telling whether its assigned nodes should be part of the focus delegation.
(Though, I wouldn't add an attribute just for that, but it should rather be something more generic which could be used for other things too in the future, at least in theory)

@mfreed7
Copy link
Contributor

mfreed7 commented Apr 4, 2024

It's a little hard for me to find the rationale for not using the flat tree - can someone summarize?

It really seems to me that the focus delegate algorithm, at least for dialogs, should just use the flat tree. Visually, in the presented example, the input is slotted into the dialog and appears there, in flat tree order. It would seem to make sense (to users!) that the first thing focused should be the first thing in the visual order.

I don't think we need an attribute or feature to control this, do we? Perhaps I don't understand the "broken" case.

@mfreed7
Copy link
Contributor

mfreed7 commented Apr 11, 2024

@domenic I had a WHATNOT action item to ping you about this question, and I dropped the ball. Here's that ping. 😄 I wasn't clear if you feel focus delegate explicitly shouldn't use the flat free, or if you were just concerned given other comments and history.

@domenic
Copy link
Member

domenic commented Apr 12, 2024

I think we decided deliberately that focus delegate for delagatesFocus should use the flat tree. #7207 in particular discussed that and made the change.

However, I understand the arguments that using the flat tree for <dialog>s is incorrect.

So my conclusion is the same as in #9245 (comment): we probably made a mistake assuming that we could use the same "focus delegate" algorithm for both delegatesFocus and <dialog>s. We should split it up into two algorithms. (Probably with very clearly distinct names, which make it clear for future spec writing which one is appropriate for which situations.)

@mfreed7
Copy link
Contributor

mfreed7 commented Apr 12, 2024

However, I understand the arguments that using the flat tree for <dialog>s is incorrect.

This is the part I don't quite yet understand though. Can you help - why is the flat tree wrong for <dialog>? I.e. in this example:

<section>
  <template shadowrootmode=open>
    <dialog>
      <slot></slot>
    </dialog>
  </template>
  <input type="text" />
</section>

The <input> is slotted into the dialog, and to the user, appears as the first/only thing inside the dialog. So it feels like a flat tree walk should be used to find and focus it.

@domenic
Copy link
Member

domenic commented Apr 12, 2024

Sorry, I misunderstood what had happened in the past and flipped the two cases. It seems we decided that focus delegate for delegatesFocus should not use the flat tree in #7207. And so the question is whether it should use the flat tree for <dialog>s, and I guess people have some arguments as to why the flat tree is better.

The conclusion is the same, though: we should split the algorithms.

@mfreed7
Copy link
Contributor

mfreed7 commented Apr 12, 2024

Sorry, I misunderstood what had happened in the past and flipped the two cases. It seems we decided that focus delegate for delegatesFocus should not use the flat tree in #7207. And so the question is whether it should use the flat tree for <dialog>s, and I guess people have some arguments as to why the flat tree is better.

Great! Ok, I do agree with not using the flat tree for delegatesFocus. Sounds like we need to hear counter-arguments for moving to the flat tree for <dialog> focus.

The conclusion is the same, though: we should split the algorithms.

Agreed, assuming someone doesn't make a good case for dialog also not using the flat tree.

@josepharhar
Copy link
Contributor

So we can use the flat tree for dialog and popover initial focus then? I can do the chromium/wpt/spec changes

@frank-topel-dbi
Copy link

frank-topel-dbi commented Jun 21, 2024

I'm not into the details of the approaches/algorithms you're using here, but please make sure autofocus inside dialogs is respected, be it slotted, or directly in the shadow root of a custom dialog web component utilizing the native <dialog> in its implementation. This crbug ticket shows that the current focusing algorithm seems to work differently based on platform (Windows and Fedora/ARM Chrome have the bug, MacOS ARM Chrome does not).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: focus topic: shadow Relates to shadow trees (as defined in DOM)
Development

No branches or pull requests

10 participants