Skip to content
This repository was archived by the owner on Sep 22, 2022. It is now read-only.

Ensure focusble targets are visible #48

Merged
merged 5 commits into from
Nov 6, 2019
Merged

Ensure focusble targets are visible #48

merged 5 commits into from
Nov 6, 2019

Conversation

muan
Copy link
Contributor

@muan muan commented Nov 5, 2019

Fixes #41, refer to considerations in #41 (comment).

This is a reliable visibility check (which comes with a perf cost as it triggers layout). I kept .hidden and type=hidden checks because they're relatively cheap and we use them often. When they were true we skip some visible calls.

Example use case:

<details-dialog>
  <details>
    <details-menu><button><!-- focusable but not visible --></button></details-menu>
  </details>
</details-dialog>

and for autofocus:

<details-dialog>
  <details>
    <details-menu><input autofocus><!-- focusable but not visible --></details-menu>
  </details>
  <button autofocus></button>
</details-dialog>

@muan muan requested a review from a team November 5, 2019 23:26
keithamus
keithamus approved these changes Nov 6, 2019
View reviewed changes
Copy link
Contributor

@keithamus keithamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@muan muan changed the title Ensure focusble targets are visaible Ensure focusble targets are visible Nov 6, 2019
@muan muan merged commit 6a07747 into master Nov 6, 2019
@muan muan deleted the visible branch November 6, 2019 18:38
@muan muan mentioned this pull request Dec 16, 2019
Merged
@theinterned theinterned mentioned this pull request Mar 1, 2022
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@keithamus keithamus keithamus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tab trapping doesn't know elements are hidden by child <details>
2 participants
@muan @keithamus