Add errorOverlayTheme option to Astro config

[MoustaphaDev]

Changes

Add errorOverlayTheme option to configure the theme of the custom error overlay

Testing

Not sure if it's worth adding an E2E test for this

Docs

JSDOCS comments, will be pulled by the docs website

[changeset-bot]

🦋 Changeset detected

Latest commit: 60a1509

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]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[bluwy]

Maybe I've missed some discussion about this, but it feels a bit "extraneous" to add this option 🤔 Is the default automatic light-dark theme not desirable? Since it's a dev-only feature, I feel like we can be a bit opinionated to keep the config API clean.

[Princesseuh]

Maybe I've missed some discussion about this, but it feels a bit "extraneous" to add this option 🤔 Is the default automatic light-dark theme not desirable? Since it's a dev-only feature, I feel like we can be a bit opinionated to keep the config API clean.

This was my opinion as well, but some people expressed that their setup (Linux people) didn't properly set the media query, so I feel like it could be harmless to add

I'd want it a bit simplified though, as right now it seems like a "major" change to the overlay code

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[MoustaphaDev]

Made a few changes to keep the error overlay code closer to what it was

[bluwy]

This was my opinion as well, but some people expressed that their setup (Linux people) didn't properly set the media query, so I feel like it could be harmless to add

I see. I guess it's nice to have a way out then, but also it feels like they should get it fixed, and not needing to workaround it. If we do want to make this possible, I wonder if it make sense to support body.astro-light and body.astro-dark instead.

[MoustaphaDev]

If we do want to make this possible, I wonder if it make sense to support body.astro-light and body.astro-dark instead.

Not sure what you're referring to here, if it can avoid changing the config object, I'd be happy to explore that path

[bluwy]

I mean if the astro-dark class is attached to the body, the error overlay would render in dark mode. (Same for light mode) Though I'm actually not sure if this behaviour would work for web components.

[delucis]

Love the initiative on this feature! In general, offering choice here is an accessibility question so finding some way to let users select their preferred theme does seem a good idea.

I wonder if the issue here is where this new option is being set. Would it make sense to offer a dark/light/system theme selector in the browser which we can persist to localstorage? The error overlay is technically a website and that’s the usual UI for this kind of option.

My personal take is that this is less a project config option and more a personal preference. Maybe two devs work on a site and have different preferences. Setting this via the config file will only help one of them. But if they can store their preference in the browser, that personal preference applies just to them.

[aFuzzyBear]

I do like this PR, but a part of me is thinking and its pure bikeshedding, but is errorOverlayTheme a suitable name for this, could we not make it a bit more generic and go with theme this way any future UI improvements or features could leverage the theme: 'dark' | 'light' | 'system' to provide a more coherent way of applying any future style changes.

[aFuzzyBear]

Love the initiative on this feature! In general, offering choice here is an accessibility question so finding some way to let users select their preferred theme does seem a good idea.

I wonder if the issue here is where this new option is being set. Would it make sense to offer a dark/light/system theme selector in the browser which we can persist to localstorage? The error overlay is technically a website and that’s the usual UI for this kind of option.

My personal take is that this is less a project config option and more a personal preference. Maybe two devs work on a site and have different preferences. Setting this via the config file will only help one of them. But if they can store their preference in the browser, that personal preference applies just to them.

I do like this suggestion, putting it behind a toggle on the overlay itself, thats worth a look, great idea there 🧠 @delucis

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[MoustaphaDev]

I wonder if the issue here is where this new option is being set. Would it make sense to offer a dark/light/system theme selector in the browser which we can persist to localstorage? The error overlay is technically a website and that’s the usual UI for this kind of option.

Thanks for your reviews @bluwy and @delucis.
@delucis, I like this idea a lot! This makes way more sense than adding an option to the user config.
I'll try to go that way

[MoustaphaDev]

Superseded by #5884