mdgriffith / elm-ui

What if you never had to write CSS again?
https://package.elm-lang.org/packages/mdgriffith/elm-ui/latest/
BSD 3-Clause "New" or "Revised" License
1.35k stars 112 forks source link

Fix Chrome focus issue #362

Open lydell opened 2 months ago

lydell commented 2 months ago

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.