Outline on button :focus

[todd-elvers]

Clicking on the toggle button produces an outline around the button in Chrome:

Adding outline: "none" to the button element fixes the issue, however that will cause confusion to anyone who is strictly using the keyboard because when they tab to the element on the page that outline won't be present. Some articles online suggest listening for key presses and, if the element is focused and the last key pressed was tab, showing the outline. While that pleases both types of users, it complicates the component.

I'm not sure what makes the most sense for this library and I defer to you @cawfree

[todd-elvers]

Here's a quick proof-of-concept of how you could implement this logic using a custom hook:

function useLastKeyPress(targetKey) {
  const [keyPressed, setKeyPressed] = React.useState(false);
  const reset = () => setKeyPressed(false);
  const downHandler = ({ key }) => {
    if (key === targetKey) {
      setKeyPressed(true);
    }
  };

  React.useEffect(() => {
    window.addEventListener("keydown", downHandler);
    window.addEventListener("click", reset);
    return () => {
      window.removeEventListener("keydown", downHandler);
      window.removeEventListener("click", reset);
    };
  }, []); // Empty array ensures that effect is only run on mount and unmount

  return keyPressed;
}

Then in the component you can add:

const wasLastKeyPressedTab = useLastKeyPress("Tab");
const outlineStyles = wasLastKeyPressedTab ? {} : { outline: "none" };

And explode those outlineStyles into the button's style attribute.

While this works it feels too heavyweight for this component and can cause it to flash while re-rendering (though that may be addressable). If you have a better solution by all means, otherwise I'm good with just addingborder: "none" to the button.

[cawfree]

Hey @todd-elvers, thanks for raising this. Honestly, you are absolutely murdering this repo (in the best way possible)!

I really like your solution, although I agree that it does overcomplicate this component, but only because it strikes me that this functionality shouldn't be isolated here; it'd probably be quite useful for other people to have a generic way to handle Chrome's outline in a modular way.

I did a quick search on npm and couldn't find a hook to use in-place which achieves this kind of functionality, although there are some providers that appear to handle this for you:

react-outline-manager

It might be easier to recommend that for Chrome support, the user wrap their application in the manager.

If you decide to opt for the custom hook route and publish a package to npm, I'd be more than happy to have the toggle depend upon it. (It'd be interesting to see if it's compatible with the provider too).

[todd-elvers]

@cawfree You're right, I believe it falls upon the user of the library to customize the outlining behavior based on the browser(s) they are targeting. That being said I'll create a PR to update the readme and make this clearer. Once I have a PR I'll mention it here.

[acatzk]

I am using tailwindcss and I only defined it using className="focus:outline-none"

here's an example
https://www.joshuagalit.ga/

[todd-elvers]

I've updated the readme to include some context and a link to this issue in PR #18