polish: bind key listener to light/dark toggle + a11y lint fixes #5341
Conversation
|
✔️ [V2] 🔨 Explore the source changes: 6a9bde8 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6115243bde91b90008aecc7f 😎 Browse the preview: https://deploy-preview-5341--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5341--docusaurus-2.netlify.app/ |
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
| @@ -39,6 +39,23 @@ const Details = ({summary, children, ...props}: DetailsProps): JSX.Element => { | |||
| // We use a separate prop because it must be set only after animation completes | |||
| // Otherwise close anim won't work | |||
| const [open, setOpen] = useState(props.open); | |||
| const toggle = (e: React.SyntheticEvent) => { | |||
There was a problem hiding this comment.
nitpick: I'd rather not extract this method, it's only used in one place and it's not even meant to be reusable considering it's handling a click dom event. Extracting the code and naming it "toggle" makes this more implicit and imply a reusable api. If you really want to extract this (like for perf reasons using useCallback which in this case wouldn't improve anything), you'd rather name it handleClick.
There was a problem hiding this comment.
No problem, I wrote this a week ago and no idea why I extracted it back then 🤦♂️
|
👍 |
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
slorber
added
the
pr: maintenance
Motivation
Fix more ESLint warnings about a11y.
Toggledeserves to have a key listener, so let's give it one (although not at the place ESLint suggests)Have you read the Contributing Guidelines on pull requests?
Yea
Test Plan
On the main page, use tab to focus the Toggle, and press enter to see the theme toggled.