Move theme switcher to account settings #10334
Conversation
lookacat
changed the base branch from
master
to
enable-preferences-for-not-logged-in-users
lookacat
force-pushed
the
move−theme−switcher−account−settings
branch
from
689fdb2 to
4686fc6
Compare
lookacat
changed the base branch from
enable-preferences-for-not-logged-in-users
to
master
AlexAndBear
changed the title
AlexAndBear
force-pushed
the
move−theme−switcher−account−settings
branch
from
cfc2ae3 to
74890b7
Compare
|
delete-merged-branch
bot
deleted the
move−theme−switcher−account−settings
branch
There was a problem hiding this comment.
Please move this file away from the Topbar folder when it's not part of the topbar anymore.
| const onUpdateTheme = () => { | ||
| showMessage({ title: $gettext('Preference saved.') }) | ||
| } |
There was a problem hiding this comment.
Why handle this here via events? I'd expect the ThemeSwitcher to handle the message 😅
There was a problem hiding this comment.
Why would you expect that?
The ThemeSwitcher doesn't know anything about the context. In that case we would expect a message like "Theme saved" or something like this.
But for account.vue we don't want to go too much into detail with the success message
There was a problem hiding this comment.
Because it seems unnecessarily complicated to me 🤷 It's not that we're updating data or something in the parent component which would justify that. I'd rather have all theme-logic decoupled in the ThemeSwitcher component (same like e.g. the GDPRExport component handles all gdpr related stuff).
Description
See #10181
Related Issue
Types of changes
Open tasks: