Repository navigation
GSTheme: follow the theme default in an application that did not change theme - #950
Conversation
…ge theme An application that started with the default theme kept it for the rest of its run, whatever the user later chose: +initialize establishes the theme by calling +defaultsDidChange:, which reaches +setTheme: only when the theme actually changes, and +setTheme: is where the observer of NSUserDefaultsDidChangeNotification is registered. An application on the default theme therefore never watched the default it was meant to follow, and only a restart picked a new theme up. This is the part of gnustep#431 that gnustep#827 left open, which it names in its own description. The observer is now registered when GSTheme initialises, before the theme named by the defaults is established. +setTheme: still removes and re-adds it around a switch, so it stays registered exactly once. Refs gnustep#431
|
I ping @rfm who worked on the previous theme switching. I did test and we have a regression, setting a theme in system preferences never changes other applications. But I am sure it was fixed, because that was the actual test. |
For what it's worth I had to do something like this to apps-systempreferences to make it work for me from System Preferences @rmottola gnustep/apps-systempreferences@master...pkgdemon:apps-systempreferences:master I am not sure what I did there is the best fix. I used Claude to come up with it in a hurry just as an experiment but here it is in action if to see what it would look like. https://www.youtube.com/watch?v=wB0PVTdTZ-g There were also some necessary fixes for apps-gworkspace to retheme some elements in real time. I am not 100% sure they are still necessary now. gnustep/apps-gworkspace@master...pkgdemon:apps-gworkspace:master Just wanted to share in case it helps reveal anything... |
rfm
left a comment
rfm
left a comment
There was a problem hiding this comment.
This remedies a trivial oversight by me: yes the GSTheme class should register itself as an observer for defaults updates when it is initialised.
Removed previous observer management for theme changes.
Remove unnecessary observer removal from notification center when changing themes.
|
But ... the code in +setTheme: was wrong: we don't need to remove (or re-register) the class as an observer for defaults changes if we have already registered it in +initialize |
|
@rfm would you like me to change this PR? If so, how? Or is it obsolete now given your commits? Thanks for looking into this. |
|
I updated/extended your PR. It still needs to be approved by Fred. And you might like to see if my changes make sense to you, or if you notice anything missed. |
5 participants
An application that starts with the default theme never follows a later change of the
GSThemedefault: the observer ofNSUserDefaultsDidChangeNotificationis registered in+setTheme:, in the branch that is taken only when the theme really changes, and at start-up+defaultsDidChange:does not reach that branch when the default names the theme which is already current. Only a restart picks a new theme up. This is the part of #431 that #827 names in its own description and leaves open.The test pins the
GSThemedefault toGNUstepbeforeGSThemeis first used, lays down a theme bundle with no code in it, switches the default to that bundle and posts the notification.Before:
After:
+initializenow registers the observer before it establishes the theme the defaults name;+setTheme:still removes and re-adds it around a switch, so it stays registered exactly once.gui/NSColor,gui/NSAppearanceandgui/NSWindowgive the same results with and without the change (45/4, 35/16 and 16/13 here).cc @pkgdemon