After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 733011 - Add application-specific theme extensions
Add application-specific theme extensions
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-07-10 14:44 UTC by Pranav Kant
Modified: 2014-07-24 12:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add application-specific theme extensions (3.92 KB, patch)
2014-07-10 14:44 UTC, Pranav Kant
none Details | Review
Add application-specific theme extensions (20.66 KB, patch)
2014-07-10 16:50 UTC, Pranav Kant
needs-work Details | Review
Add application-specific theme extensions (27.40 KB, patch)
2014-07-16 14:32 UTC, Debarshi Ray
committed Details | Review
theme: Add CSS snippet for GdMainIconView (864 bytes, patch)
2014-07-18 14:41 UTC, Debarshi Ray
committed Details | Review
build: Add Adwaita.css to EXTRA_DIST (649 bytes, patch)
2014-07-23 11:31 UTC, Debarshi Ray
committed Details | Review
theme: Add documents-entry-tag style class (1.43 KB, patch)
2014-07-24 12:11 UTC, Debarshi Ray
committed Details | Review
theme: Add missing colour definitions (715 bytes, patch)
2014-07-24 12:12 UTC, Debarshi Ray
committed Details | Review

Description Pranav Kant 2014-07-10 14:44:14 UTC
Based on code from patches to Bug 732995
Comment 1 Pranav Kant 2014-07-10 14:44:19 UTC
Created attachment 280413 [details] [review]
Add application-specific theme extensions
Comment 2 Pranav Kant 2014-07-10 16:50:41 UTC
Created attachment 280424 [details] [review]
Add application-specific theme extensions
Comment 3 Debarshi Ray 2014-07-16 13:12:29 UTC
Review of attachment 280424 [details] [review]:

Thanks for the patch, Pranav.

::: data/Makefile.am
@@ +20,3 @@
+	sidebar-radio-prelight.svg \
+	sidebar-radio-selected-prelight.svg \
+	sidebar-radio-selected.svg \

These should go to EXTRA_DIST. The fact that thumbnail-frame.png is still there is a mistake. It should have been moved in b09166df935d79c226fc2fa58bd2da2ffb243bf9

The reason being that all these files are embedded into the gnome-photos binary using GResource. Therefore there is no need to install them separately. We only need to have them in the tarball.

::: src/photos-application.c
@@ +738,3 @@
+  static GtkCssProvider *provider;
+
+  if (pspec == NULL || g_str_equal (pspec->name, "gtk-theme-name"))

If we connect to notify::gtk-theme-name then we don't need this.

@@ +781,3 @@
+  settings = gtk_settings_get_default ();
+  g_signal_connect (settings, "notify", G_CALLBACK (theme_changed), NULL);
+  theme_changed (settings, NULL, NULL);

We can move these two lines to photos_application_startup where we set the dark theme.

::: src/photos-preview-view.c
@@ +199,3 @@
   gtk_scrolled_window_set_shadow_type (GTK_SCROLLED_WINDOW (self), GTK_SHADOW_IN);
   context = gtk_widget_get_style_context (GTK_WIDGET (self));
+  gtk_style_context_add_class (context, "photos-scrolledwin");

I think this needs to stay 'documents-scrolledwin' for the moment because it is also used inside libgd, which is shared across gnome-documents, gnome-photos and others.

Maybe we can rename it to 'gd-scrolledwin', but that would need changes to libgd. Feel free to file a bug against libgd.
Comment 4 Debarshi Ray 2014-07-16 13:51:51 UTC
Review of attachment 280424 [details] [review]:

::: data/Adwaita.css
@@ +50,3 @@
+    border-width: 1px 0 0;
+    border-radius: 0;
+}

Should be documents-scrolledwin, as noted elsewhere. We should also carry the documents-counter class because GdMainView can use it.

@@ +53,3 @@
+
+.photos-collection-icon {
+    border-radius: 8px;

We are missing background-color, which causes the collection icons to have transparent backgrounds.
Comment 5 Debarshi Ray 2014-07-16 14:29:40 UTC
For some reason GdTogglePixbufRenderer is not rendering the check boxes when an item is right-clicked. It is done using standard gtk+ style classes and gtk_render_check so probably not something we need to worry about in the application.
Comment 6 Debarshi Ray 2014-07-16 14:32:47 UTC
Created attachment 280842 [details] [review]
Add application-specific theme extensions

Addressed the issues in comment 3 and comment 4
Comment 7 Debarshi Ray 2014-07-18 14:41:56 UTC
Created attachment 281103 [details] [review]
theme: Add CSS snippet for GdMainIconView

As pointed out by Cosimo.
Comment 8 Debarshi Ray 2014-07-23 11:31:32 UTC
Created attachment 281469 [details] [review]
build: Add Adwaita.css to EXTRA_DIST
Comment 9 Debarshi Ray 2014-07-24 12:11:39 UTC
Created attachment 281575 [details] [review]
theme: Add documents-entry-tag style class
Comment 10 Debarshi Ray 2014-07-24 12:12:11 UTC
Created attachment 281576 [details] [review]
theme: Add missing colour definitions