Closed aptkingston closed 1 month ago
QA Wolf here! As you write new code it's important that your test coverage is keeping up. Click here to request test coverage for this PR!
Just sign-off on the few changes in
packages/server
, haven't reviewed the majority of stuff but don't want you getting stuck on CODEOWNER approval. 🙂
Cheers Sam. Added a few basic sanity tests for the shared-core utils there too.
LGTM
Just wondering if there's actually any need to have logic to perform the fallbacks etc. Can we just remove the option to select it as a theme, and keep it around behind the scenes for apps/builders that currently use it?
I would have, but it presents a few problems:
lightest
and spectrum--lightest
for builder and app respectively), which have now been unifiedI've tried to alleviate the pain of that by moving to the closest match though, so lightest will become light, and dark will become darkest. So it's not as if there should be huge visual differences at least.
If you think it's too much to force migrate them at all though then we can shelve this for now. Doesn't need to go in.
Description
This PR removes the lightest and dark theme options.
I've also cleaned up and improved our management of themes in the codebase, adding a proper enum and type for metadata, moving utils into shared-core and using them across the board.
We previously stored themes different in the builder and client app (sometimes prefixed with
spectrum--
and sometimes not) and the code was littered with magic strings. We now also have proper constants for the available and default themes for apps and the builder.In terms of backwards compatibility: