Merge lp:~larsu/unity-control-center/add-overlay-scrollbar-key into lp:unity-control-center

Proposed by Lars Karlitski
Status: Rejected
Rejected by: Sebastien Bacher
Proposed branch: lp:~larsu/unity-control-center/add-overlay-scrollbar-key
Merge into: lp:unity-control-center
Diff against target: 71 lines (+11/-12)
1 file modified
panels/appearance/cc-appearance-panel.c (+11/-12)
To merge this branch: bzr merge lp:~larsu/unity-control-center/add-overlay-scrollbar-key
Reviewer Review Type Date Requested Status
Lars Karlitski (community) Disapprove
Robert Ancell Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+227758@code.launchpad.net

Commit message

appearance: introduce X-Ubuntu-UseOverlayScrollbars

So that theme authors can opt in to using overlay scrollbars.

Description of the change

appearance: introduce X-Ubuntu-UseOverlayScrollbars

So that theme authors can opt in to using overlay scrollbars.

This should land at the same time as lp:~larsu/ubuntu-themes/use-overlay-scrollbars. Otherwise, overlay-scrollbars will be turned off for Ambiance and Radiance as well.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

Looks good, but why did you remove the g_settings_delay / g_settings_apply?

review: Needs Information
Revision history for this message
Lars Karlitski (larsu) wrote :

> Looks good, but why did you remove the g_settings_delay / g_settings_apply?

Because canonical_interface_settings has only ever one key set (the overlay-scrollbar one). delay/apply only makes sense when you set multiple keys at once (to only trigger one write of the dconf db) or when you have a dialog that is not instant-apply.

Revision history for this message
Robert Ancell (robert-ancell) :
review: Approve
Revision history for this message
Lars Karlitski (larsu) wrote :

This is actually not the right place for deciding whether to show overlay-scrollbars. Instead, the overlay scrollbar module should look at the key and only enable itself when the theme supports this. The gsettings key can then be used as a manual override for users.

I've done this at https://code.launchpad.net/~larsu/overlay-scrollbar/only-enable-for-supported-themes

Thanks to Ryan for bringing this to my attention.

review: Disapprove

Unmerged revisions

12786. By Lars Karlitski

appearance: introduce X-Ubuntu-UseOverlayScrollbars

So that theme authors can opt in to using overlay scrollbars.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'panels/appearance/cc-appearance-panel.c'
2--- panels/appearance/cc-appearance-panel.c 2014-07-02 09:42:52 +0000
3+++ panels/appearance/cc-appearance-panel.c 2014-07-22 14:48:59 +0000
4@@ -1261,7 +1261,8 @@
5 gchar **gtk_theme,
6 gchar **icon_theme,
7 gchar **window_theme,
8- gchar **cursor_theme)
9+ gchar **cursor_theme,
10+ gboolean *use_overlay_scrollbars)
11 {
12 gchar *path;
13 GKeyFile *theme_file;
14@@ -1279,6 +1280,9 @@
15 *window_theme = g_key_file_get_string (theme_file, "X-GNOME-Metatheme", "MetacityTheme", NULL);
16 *cursor_theme = g_key_file_get_string (theme_file, "X-GNOME-Metatheme", "CursorTheme", NULL);
17
18+ if (use_overlay_scrollbars)
19+ *use_overlay_scrollbars = g_key_file_get_boolean (theme_file, "X-GNOME-Metatheme", "X-Ubuntu-UseOverlayScrollbars", NULL);
20+
21 result = TRUE;
22 }
23 else
24@@ -1298,33 +1302,28 @@
25 {
26 gint active;
27 gchar *gtk_theme, *icon_theme, *window_theme, *cursor_theme;
28+ gboolean use_overlay_scrollbars;
29
30 active = gtk_combo_box_get_active (combo);
31 g_return_if_fail (active >= 0 && active < G_N_ELEMENTS (themes_id));
32
33 if (!get_theme_data (gtk_combo_box_get_active_id (combo),
34- &gtk_theme, &icon_theme, &window_theme, &cursor_theme))
35+ &gtk_theme, &icon_theme, &window_theme, &cursor_theme, &use_overlay_scrollbars))
36 return;
37
38 g_settings_delay (self->priv->interface_settings);
39
40- if (self->priv->canonical_interface_settings != NULL)
41- g_settings_delay (self->priv->canonical_interface_settings);
42-
43 g_settings_set_string (self->priv->interface_settings, "gtk-theme", gtk_theme);
44 g_settings_set_string (self->priv->interface_settings, "icon-theme", icon_theme);
45 g_settings_set_string (self->priv->interface_settings, "cursor-theme", cursor_theme);
46 g_settings_set_string (self->priv->wm_theme_settings, "theme", window_theme);
47
48-
49- /* disable overlay scrollbars for a11y if installed*/
50 if (self->priv->canonical_interface_settings)
51 {
52- if (g_strcmp0 (gtk_theme, "HighContrast") == 0 )
53+ if (use_overlay_scrollbars)
54+ g_settings_reset (self->priv->canonical_interface_settings, "scrollbar-mode");
55+ else
56 g_settings_set_string (self->priv->canonical_interface_settings, "scrollbar-mode", "normal");
57- else
58- g_settings_reset (self->priv->canonical_interface_settings, "scrollbar-mode");
59- g_settings_apply (self->priv->canonical_interface_settings);
60 }
61
62 g_settings_apply (self->priv->interface_settings);
63@@ -1370,7 +1369,7 @@
64 gchar *gtk_theme, *icon_theme, *window_theme, *cursor_theme, *new_theme_name;
65 GtkTreeIter iter;
66
67- if (!get_theme_data (themes_id[i], &gtk_theme, &icon_theme, &window_theme, &cursor_theme))
68+ if (!get_theme_data (themes_id[i], &gtk_theme, &icon_theme, &window_theme, &cursor_theme, NULL))
69 {
70 current_theme_index--;
71 continue;

Subscribers

People subscribed via source and target branches