Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add richer card theme support #1809

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: Add richer card theme support #1809

wants to merge 6 commits into from

Conversation

dermotduffy
Copy link
Owner

@dermotduffy dermotduffy commented Jan 5, 2025

BREAKING CHANGE: Alters the behavior and renames the previous view.dark_mode parameter to view.dim. Now behaves separately from choice of theme.

BREAKING CHANGE: Alters the behavior and renames the previous `view.dark_mode` parameter to `view.dim`. Now behaves separately from choice of theme.
@github-actions github-actions bot added the feature New feature or request label Jan 5, 2025
@dermotduffy dermotduffy changed the title feat: Add richer card theme support. feat: Add richer card theme support! Jan 5, 2025
@dermotduffy dermotduffy changed the title feat: Add richer card theme support! feat!: Add richer card theme support Jan 5, 2025
@dermotduffy dermotduffy changed the title feat!: Add richer card theme support feat: Add richer card theme support Jan 5, 2025
@dermotduffy
Copy link
Owner Author

@felipecrs If you have time, would you mind trying this out? It's changes the theming of the default card, so a second pair of eyes would be good.

view:
  theme:
    themes:
      - ha               // <--- Can set this value to any of ['ha', 'dark', 'light', 'traditional'].

@felipecrs
Copy link
Contributor

I do not see any difference:

chrome_D0SaXcqN0M.mp4

Is this expected?

@felipecrs
Copy link
Contributor

Oh, never mind! It was a caching issue.

@felipecrs
Copy link
Contributor

felipecrs commented Jan 6, 2025

But is this expected?

chrome_NBXmTHaMji.mp4

I suppose traditional was meant to make it look more like the prior look. Indeed it changes the background color of the menu, but the buttons are a bit weird, witheish.

@dermotduffy
Copy link
Owner Author

Thanks for testing! Yes, I think that looks roughly as intended:

  • ha: Just lets HA set the styles (if you toggle light or dark in your HA settings, the card theme will change to match)
  • light: Akin to the above when HA theme is not dark.
  • dark: Identical to the above when HA theme is dark.
  • traditional: Similar to the currently released styling (although not identical).

When in ha (the default), the menu/status background/foreground should match the HA "tab" listing foreground/background.

@dermotduffy
Copy link
Owner Author

I'll add: It might not seem like a big difference, but this lets us define entirely new themes in future with minimal changes, and allows for overrides of CSS parameters within a theme (e.g. "I want my menu red but everything else the same").

I'm mostly concerned with getting the default theme to look well :-)

@felipecrs
Copy link
Contributor

felipecrs commented Jan 6, 2025

I'm mostly concerned with getting the default theme to look well :-)

Right.

Maybe you can tweak traditional to look more like the previous version of the card, and make it the default.

This would allow users a more seamless transition, while still giving them the chance of trying the new theme and provide feedback.

@felipecrs
Copy link
Contributor

felipecrs commented Jan 6, 2025

Or, if you are keen to, to prevent a breaking change (and thus a new major release), you can retain support for the view.dark_mode for the time being and only remove/migrate it in a separate PR that perhaps combines additional breaking changes.

@dermotduffy
Copy link
Owner Author

@felipecrs Changed traditional to the default, made it closer to what we have today. Any better?

Or, if you are keen to, to prevent a breaking change (and thus a new major release), you can retain support for the view.dark_mode for the time being and only remove/migrate it in a separate PR that perhaps combines additional breaking changes.

100% understand the spirit of that question, but I think if we're to embrace the conventional commits / semantic releases approach, we need to lose the "romantic" meaning of major version change, and stick with just the backwards-incompatible meaning. No point in hiding from it, even though I am sure people will complain :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Icons Not Illuminating Add dark and light theme support Entity based icons should all support state_color
2 participants