Skip to content

Conversation

@lydell
Copy link

@lydell lydell commented Aug 19, 2024

Chrome is rolling out a change that affects elm-ui: https://developer.chrome.com/blog/keyboard-focusable-scrollers

The new thing is that scrollable elements can be focused now (in some situations), to aid keyboard using people – they can now focus scrollable elements to then scroll them using the keyboard.

To test it:

  1. Update to Chrome 127
  2. Make sure that the feature is enabled by visiting chrome://flags/#keyboard-focusable-scrollers
  3. Visit https://ellie-app.com/rY2YmBFd44pa1
  4. Click inside the scrollable area (but not on a button). This gives focus to the scrollable area.

Problem: All buttons inside get their focus styling.

This happens because elm-ui’s focus CSS looks like this.

.s:focus .focusable,
.s.focusable:focus {
   /* focus styles here */
}

Let’s break those selectors down. We’ll start with the second part.

.s.focusable:focus is easy to understand: When a focusable element gets focus, it is styled.

.s:focus .focusable is more difficult. It says that if a certain parent element has focus, all focusable elements inside get their focus styles. This is causing the issue. By reading the code, the only place where this is needed seems to be checkboxes and radio groups. In those cases, a <label class="s"> parent element gets focus, but an element inside (the actual checkbox (square) or radio (circle)) is supposed to get the styles.

This PR changes the first selector to label.s:focus .focusable, which solves the problem with scrollable .s elements giving styles to all its focusable children. (It is very unlikely to use a <label> element in that situation). This depends on that my reading of the code is correct, that it is only ever <label> elements that this CSS selector should start at.

lydell added a commit to insurello/elm-ui that referenced this pull request Feb 5, 2025
This applies mdgriffith#362

Conflicts:
	src/Internal/Model.elm
@lydell lydell closed this Feb 6, 2025
@cdevienne
Copy link

@lydell did you close the PR on purpose ? I have a feeling the issue is still valid and need fixing in elm-ui itself.

@lydell lydell reopened this Feb 26, 2025
@lydell
Copy link
Author

lydell commented Feb 26, 2025

Yes, I closed it by mistake. Thanks for letting me know.

@cdevienne
Copy link

@mdgriffith Sorry to bother you with this but is there any chance this gets into a release in a not-too-far release ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants