Skip to content

Redesign frontend.set_theme service form#157866

Merged
MartinHjelmare merged 9 commits into
home-assistant:devfrom
karwosts:two_theme_selectors
Dec 22, 2025
Merged

Redesign frontend.set_theme service form#157866
MartinHjelmare merged 9 commits into
home-assistant:devfrom
karwosts:two_theme_selectors

Conversation

@karwosts
Copy link
Copy Markdown
Member

@karwosts karwosts commented Dec 3, 2025

Proposed change

The current service schema for frontend.set_theme I think is not very intuitive, and I think it's misleading to a lot of users.
image

I believe when users see the radio buttons for light/dark, they intuitively think that calling this service can somehow change their UI to dark or light mode, and they get confused when calling this service seems to have no effect, e.g. because their client is set to light mode, and they keep changing the dark mode theme which doesn't affect them. Which I don't really blame them because the descriptions on this screen doesn't really explain what is happening.

I think it would be much more intuitive here if we just show two separate theme selectors (and clearly describe their use):

image

Then I think it's much clearer to understand that this service does not influence the light/dark setting of a client.

The old schema will still work the same, this just adds a new schema that can set one or both modes in a single call.

Will update docs on approval.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • I understand the code I am submitting and can explain how it works.
  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@home-assistant
Copy link
Copy Markdown
Contributor

home-assistant Bot commented Dec 3, 2025

Hey there @home-assistant/frontend, mind taking a look at this pull request as it has been labeled with an integration (frontend) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of frontend can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign frontend Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@rareruby219

This comment was marked as off-topic.

@rareruby219

This comment was marked as off-topic.

Copy link
Copy Markdown
Contributor

@NoRi2909 NoRi2909 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very welcome improvement, the current Ui is really misleading. Two small suggestions below.

Comment thread homeassistant/components/frontend/strings.json Outdated
Comment thread homeassistant/components/frontend/strings.json Outdated
Comment thread homeassistant/components/frontend/strings.json Outdated
Co-authored-by: Norbert Rittel <norbert@rittel.de>
Comment thread homeassistant/components/frontend/__init__.py Outdated
Comment thread homeassistant/components/frontend/__init__.py Outdated
balloob
balloob previously requested changes Dec 4, 2025
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. Left some comments with improvements.

@home-assistant
Copy link
Copy Markdown
Contributor

home-assistant Bot commented Dec 4, 2025

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@home-assistant home-assistant Bot marked this pull request as draft December 4, 2025 04:22
@karwosts
Copy link
Copy Markdown
Member Author

karwosts commented Dec 4, 2025

Changed to raise ServiceValidationError instead of logging a warning for a nonexistant theme. I was hesitant to do this at first as it seems like a maybe slightly breaking change, but I do agree it's better this way.

I don't think it can be checked in the schema as we don't have hass?

@karwosts

This comment was marked as outdated.

@karwosts karwosts marked this pull request as ready for review December 4, 2025 14:05
@home-assistant home-assistant Bot requested a review from balloob December 4, 2025 14:05
@piitaya
Copy link
Copy Markdown
Member

piitaya commented Dec 4, 2025

I really like the change and I wonder if we can go futher by completley removing the dark theme (at least from the UI).
Do we need to support 2 different themes backend side? You can define an unique theme with light and dark variation in it.
Also, why do we allow light and dark theme at system level but not at browser level?

This not directly related but I would personally prefer :

System :

  • unique theme
  • mode (light/dark)

Browser :

  • unique theme
  • mode (system/auto/light/dark)

I know that many users want to automate light and dark modes using automation instead of relying on computer/phone mode.

@karwosts
Copy link
Copy Markdown
Member Author

karwosts commented Dec 4, 2025

If I was designing from scratch today it might make sense to do it that way, but that's a non-trivial breaking change if you can't use two different themes anymore? Maybe if you phase it in with a deprecation period it's ok.

@karwosts
Copy link
Copy Markdown
Member Author

karwosts commented Dec 4, 2025

I think we could have a system-default light/dark mode, though I hesitate due to the loading splash screen... would we be able to figure out the mode before we flash a light or dark mode loading screen? I think people will complain about that.

Actually do we currently not even respect the user configured preference for the loading screen, we just use the OS default? Maybe that's a non-issue then.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frontend team should approve this PR before we merge.

Comment thread homeassistant/components/frontend/__init__.py Outdated
Comment thread homeassistant/components/frontend/__init__.py Outdated
@home-assistant home-assistant Bot marked this pull request as draft December 5, 2025 10:13
@MartinHjelmare
Copy link
Copy Markdown
Member

I don't think it can be checked in the schema as we don't have hass?

We can get hass nowadays in custom validator functions used in the validation schema.

Example:

def _deprecated_platform(value: str) -> str:
"""Validate if platform is deprecated."""
if value == DOMAIN:
_LOGGER.warning(
"The cloud tts platform configuration is deprecated, "
"please remove it from your configuration "
"and use the UI to change settings instead"
)
hass = async_get_hass()
async_create_issue(
hass,
DOMAIN,
"deprecated_tts_platform_config",
breaks_in_ha_version="2024.9.0",
is_fixable=False,
severity=IssueSeverity.WARNING,
translation_key="deprecated_tts_platform_config",
)
return value

vol.Required(CONF_PLATFORM): vol.All(cv.string, _deprecated_platform),

@MartinHjelmare MartinHjelmare marked this pull request as draft December 9, 2025 13:33
@MartinHjelmare
Copy link
Copy Markdown
Member

Marking as draft for now.

@wendevlin do you want @karwosts to keep this PR open for now, and/or get involved in some other way? Will you come back with more feedback here or should we close here now and make a new PR when the designs and the project are ready for that?

@wendevlin
Copy link
Copy Markdown
Member

Marking as draft for now.

@wendevlin do you want @karwosts to keep this PR open for now, and/or get involved in some other way? Will you come back with more feedback here or should we close here now and make a new PR when the designs and the project are ready for that?

Thank you,

I think we can close it for now. I ping @matthiasdebaat who will work on the designs, please keep @karwosts in the loop

@github-actions github-actions Bot locked and limited conversation to collaborators Dec 10, 2025
@MartinHjelmare
Copy link
Copy Markdown
Member

I'm reopening by request from @matthiasdebaat.

@home-assistant home-assistant unlocked this conversation Dec 16, 2025
@matthiasdebaat
Copy link
Copy Markdown
Member

Thanks for reopening @MartinHjelmare and thanks @karwosts for this PR.

When we talked about this PR last week, we came up with a long term solution that needs a bigger design concept. In summary it is aligning the profile page and services, with light/dark modes and an override for specific dark mode.

This PR is a great intermediate solution as it already makes things better. I have a couple of suggestions:

  • Remove the Theme checkbox so that it always set this one.
  • Change dark theme label to Dark theme override. As this is an alternative dark theme.

Other text changes:

  • Change the service name to Set the theme.
  • Change description to Sets the theme Home Assistant uses. Can be overridden by a user..
  • Change the theme description to Name of the theme that is used by default..
  • Change the dark theme override description to Alternative dark-mode theme that is used by default..

@karwosts what do you think? Feel free to suggest changes.

@karwosts
Copy link
Copy Markdown
Member Author

karwosts commented Dec 16, 2025

Remove the Theme checkbox so that it always set this one.

I'm not too sure about this. Prior this PR you can update light, or update dark. And whatever one you don't update is unchanged.

Now you suggest you should no longer be able to update dark without also updating light.

But you can update light without updating dark? If you do that is dark supposed maintain its existing setting, or reset to unchanged? My code currently keeps the previous setting but I'm not sure if that's what you want.

So I think it make sense to do either:

  • Keep this PR as it is (then it is a non-breaking change which can do perfect migration)
    or
  • Change so that previous dark setting is not maintained: You can set light only (in which case dark theme is reset to none) or you can set a combo light/dark selection. If we do this we have to also change the key names or else it will be a breaking change, which I'm not so thrilled about.

Or if its too confusing we can just wait for your redesign.

@matthiasdebaat
Copy link
Copy Markdown
Member

I’m okay with keeping the checkbox.

With a few small text changes, I think we can make it clearer what this service does. For example, by calling the dark theme selector an override.

@karwosts
Copy link
Copy Markdown
Member Author

Updated the strings to your proposal.

I am a little unsure that you asked to remove reference to "dark-mode client", as I wanted to emphasize that it's the client that sets the mode, not the service; but I'm fine with your proposal as well if you think that's unnecessary.

@karwosts karwosts marked this pull request as ready for review December 17, 2025 14:29
Comment thread homeassistant/components/frontend/strings.json Outdated
@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Dec 17, 2025

There's a change request above.

#157866 (comment)

@MartinHjelmare MartinHjelmare marked this pull request as draft December 17, 2025 20:01
@karwosts
Copy link
Copy Markdown
Member Author

There's a change request above.

I received the desired strings directly from the UX lead, so any further changes should be deferred to reviewers, it's not for me to decide.

@karwosts karwosts marked this pull request as ready for review December 17, 2025 20:06
@MartinHjelmare MartinHjelmare marked this pull request as draft December 18, 2025 13:51
@karwosts
Copy link
Copy Markdown
Member Author

Resolved the request

@karwosts karwosts marked this pull request as ready for review December 18, 2025 13:52
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MartinHjelmare MartinHjelmare dismissed balloob’s stale review December 18, 2025 13:57

Comments addressed

@MartinHjelmare MartinHjelmare merged commit 9ce46c0 into home-assistant:dev Dec 22, 2025
66 checks passed
@karwosts karwosts deleted the two_theme_selectors branch December 22, 2025 22:23
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Misleading behaviour of frontend's theme change service

8 participants