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

Dialog initial focus when dialog contains a slot element #9640

Closed
josepharhar opened this issue Aug 26, 2023 · 1 comment
Closed

Dialog initial focus when dialog contains a slot element #9640

josepharhar opened this issue Aug 26, 2023 · 1 comment
Labels
topic: focus topic: shadow Relates to shadow trees (as defined in DOM)

Comments

@josepharhar
Copy link
Contributor

I am (unfortunately) still working on shipping the new dialog initial focus algorithm in chrome, and I am running into breaking changes in chrome's webui (chrome://) pages.

One of the pages has a complex custom element and shadowdom structure that sort of looks like this:

<cr-dialog>
  #shadow-root 
    <dialog>
     <slot></slot>
    </dialog>
  <print-preview-search-box> (slotted)
    #shadow-root 
      <cr-input>
        #shadow-root
          <input autofocus>
      </cr-input>
  </print-preview-search-box>
</cr-dialog>

The page wants <input autofocus> to receive initial focus when the dialog is opened. This currently happens with the old code in chrome because the dialog element just does a flat tree traversal until it finds a descendant with autofocus.

In order to make this work with the new focus delegate spec, I added delegatesFocus to every shadow root and I added the autofocus attribute to every shadow host. However, it still doesn't work, and I think it's because the input we want to autofocus is being slotted into the dialog. @domenic is there any proper way that the input could get initial focus with this structure? Would putting autofocus on the slot make any sense at all? If not, I guess I'll have to find a way to get a hold of the input element in script and focus it manually.

Relevant issues/PRs:
#7079
#7284
#7361
#8174
#8156
#8157
#8199

@annevk annevk added topic: shadow Relates to shadow trees (as defined in DOM) topic: focus labels Aug 28, 2023
@domenic
Copy link
Member

domenic commented Aug 30, 2023

This basically seems to be a duplicate of #9245. As noted there,

Maybe using a single "focus delegate" algorithm for both cases was too ambitious?

I've lost most of the context here and can no longer help with concrete suggestions for improving the algorithm. But it seems likely we should split "focus delegate" into "shadow host focus delegate" (the current algorithm, although we could remove some of the dialog special stuff) and "dialog focus delegate" (some new algorithm, which uses... the flat tree?? I dunno).

Let's close this and discuss in #9245, although your example is helpful.

@domenic domenic closed this as completed Aug 30, 2023
aarongable pushed a commit to chromium/chromium that referenced this issue Aug 26, 2024
This pulls out one bit of the DialogNewFocusBehavior that is
specific to popovers, and is per-spec. Since this shouldn't
be a breaking change, this just fixes the bug and adds a
killswitch in case of compat issues. But there shouldn't be
any issues with this.

This fixes the last three popover Interop2024 tests:
https://wpt.fyi/results/html/semantics/popovers/popover-focus.html?label=stable

See this discussion for the more general dialog focus algorithm:
 whatwg/html#9640

Bug: 40369887
Change-Id: I2b197293f83d76ffc1834429e77dadff57b5b587
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5809090
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1346786}
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

3 participants