GNOME Bugzilla – Bug 630428
support for changing theme
Last modified: 2011-01-30 21:02:53 UTC
Created attachment 170928 [details] [review] add support for changing theme Add 'theme-path' option. If it's value empty, default theme will be used.
Created attachment 170932 [details] [review] add support for changing theme
Created attachment 177513 [details] [review] gnome-shell-bgo630428-change-theme.diff I rebased Maxim's patch for the latest sources. I also added an "rt" command for the Run dialog, short for "reload theme". This makes it very quick to experiment with changes to the CSS file, for example.
Created attachment 177515 [details] [review] gnome-shell-bgo630428-reload-theme.diff Alternative version that doesn't add a "theme-path" settings key. This just adds a "rt" command in the Run dialog.
Created attachment 177530 [details] [review] Factor out a function to load the default theme We will use this function elsewhere when the theme needs to be reloaded.
Created attachment 177531 [details] [review] Add an 'rt' command to the Run dialog to reload the theme This should be useful for theme authors who want to quickly reload the theme without restarting the whole shell. Signed-off-by: Federico Mena Quintero <federico@gnome.org>
Review of attachment 177530 [details] [review]: Looks good
Review of attachment 177531 [details] [review]: Sure!
Thanks! Both commits pushed to master. Should I leave the bug open for the part about actually changing the current theme?
(In reply to comment #8) > Thanks! Both commits pushed to master. > > Should I leave the bug open for the part about actually changing the current > theme? Hmm, no. Switching theme isn't currently something we are planning for and the CSS classnames and details of the actor hierarchy aren't something we want to expose as publicly supportable yet. Any initial support for theme switching will almost certainly be just for high-contrast and inverse accessibility themes so the approach of pointing to an absolute filename out of a config key isn't right. I think it's probably easiest to just WONTFIX this - we might eventually want something but keeping this open doesn't seem worth while.
Forgot to actually WONTFIX
*** Bug 640821 has been marked as a duplicate of this bug. ***
Created attachment 179561 [details] [review] Add a global themes var to Main A little change to the main.js which permits extensions to specify a hash that is passed to the StTheme constructor (then to libcroco one). Maybe some checks could be useful but I can't exactly identify which ones.
Created attachment 179649 [details] [review] Add an API to set the theme This adds a function to set a theme. Set it to null will reset to the default Gnome-Shell theme.
[ Reopening bug since I said I'd take a simple patch to let an extension override the global stylesheet ]
Review of attachment 179649 [details] [review]: Your commit message needs a body - see the email about writing commit messages linked to in the first section of https://live.gnome.org/GnomeShell/Development/WorkingWithPatches +let _default_theme = null; +let _theme = null; Note the contrast with the theme variable name used elsewhere and the "StTheme" object - these clearly aren't the theme and should be called something like "_default_css_stylesheet" "_css_stylesheet" /** + * setTheme + * + * Set the theme file that the shell will load + */ +function setTheme(theme) +{ + if ( theme ) + _theme = theme; +} - Please indent with 4 spaces. No tabs in the JS files - No spaces around (theme) - This doesn't work as intended since this function does nothing if theme is null - Document that setting to null restores to the default theme in the doc comment here + if (_theme == null) + _theme = _default_theme; It's better to use a local variable than to modify the global variable.
Created attachment 179650 [details] [review] Add an API to set the theme Fixed patch with full doc comment and a commit body.
Created attachment 179651 [details] [review] Add an API to set the theme Fixed patch with full doc comment and a commit body. (Oops)
Comment on attachment 179651 [details] [review] Add an API to set the theme Pushed as 5abf9a04255d1b6f2b7c004221ada7e01eb3ae39
Closing as WONTFIX again since this still isn't a "standard" way to change the theme just a hook for extensions.