feat: add theme toggle button to error overlay #5884
Conversation
🦋 Changeset detectedLatest commit: dafd26e The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: astro
| // theme toggle logic | ||
| const themeToggleButton = this.root.querySelector<HTMLButtonElement>('#theme-toggle'); | ||
| if ( | ||
| localStorage.astroErrorOverlayTheme === 'dark' || |
There was a problem hiding this comment.
Not sure if it would've been an issue, but I made the name of the key this lengthy so that it'd be unlikely to coincide with a key already in local storage that the developer had set in their main site.
|
Liking this enhancement! Just popping in with some design guidance. The placement on the left side above the main title is very prominent in the hierarchy. I mocked up a more standard toggle switch with it less prominent in the top right here: Let me know if I can provide any other guidance. |
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
|
Here's the final result
|
There was a problem hiding this comment.
That's a great addition to the error overlay, nice work @MoustaphaDev! There are a few a11y issues I made suggestions to fix:
- Missing label text (so screen readers can explain that the checkbox activates dark mode)
Before: "Checkbox (checked/not checked)"
After: "Use dark theme, checkbox (checked/not checked)"
Tested with NVDA, different screen readers can differ here.
- No focus styles
| Before (Focused) | After (Focused) |
|---|---|
 |
 |
- Functionality unclear in high contrast mode
| Before | After |
|---|---|
 |
 |
MoustaphaDev
changed the title
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
|
Thanks for the awesome a11y checks @Yan-Thomas! |
Changes
Add a theme toggle button for the error overlay
Testing
Tested manually
Docs
This is a style change, doesn't need to be documented