Created attachment 281547 [details] [review]
Add application-specific theme extensions

A very rough patch. I am yet to get the correct background. A review will be appreciated so that I know if I am working in the right direction.

Review of attachment 281547 [details] [review]:

Thanks for the patch, Saurav. I have not tried it out, but, yes, you are in the right direction.

Review of attachment 281547 [details] [review]:

::: src/resources/Adwaita.css
@@ +29,3 @@
+.documents-dropdown .view.radio:selected {
+    background-image: none;
+    background-color: alpha(@theme_base_color, 0.0);

Don't you have to define theme_base_color? I think this is why the selected dots are not showing up in the dropdown.

@@ +86,3 @@
+.documents-favorite.button:active,
+.documents-favorite.button:active:hover {
+    color: shade(@theme_selected_bg_color, 1.20);

Same for theme_selected_bg_color.

@@ +91,3 @@
+.documents-entry-tag {
+    background-color: @entry_tag_bg;
+    color: @entry_tag_fg;

Ditto for entry_tag_bg and entry_tag_fg. This could be why the tags are completely white / transparent.

@@ +123,3 @@
+    border-width: 3px 3px 6px 4px;
+    border-image: url("thumbnail-frame.png") 3 3 6 4;
+}

Interesting. We have something similar done in code (see src/documents.js). I wonder if that can be completely replaced by CSS.

@@ +132,3 @@
+    /* when there's no pixbuf yet */
+    background-color: @osd_bg;
+}

The 'osd' style class should be part of Adwaita core and in gtk+. Do we really need this?

(In reply to comment #3)

> @@ +123,3 @@
> +    border-width: 3px 3px 6px 4px;
> +    border-image: url("thumbnail-frame.png") 3 3 6 4;
> +}
> 
> Interesting. We have something similar done in code (see src/documents.js). I
> wonder if that can be completely replaced by CSS.

I am not clear on this. So, I am including this for now. 

> @@ +132,3 @@
> +    /* when there's no pixbuf yet */
> +    background-color: @osd_bg;
> +}
> 
> The 'osd' style class should be part of Adwaita core and in gtk+. Do we really
> need this?

I saw that it has been included in gtk+ but ".osd .page-thumbnail" was mentioned as being used by Documents in gtk-widgets.css in gnome-themes-standard. I couldn't find ".osd .page-thumbnail" in gtk+ so thought of including it.

Created attachment 281611 [details] [review]
Add application-specific theme extensions

Defined the required colors. The patch might need more work since I am unable to get the required background color instead of white.

Review of attachment 281611 [details] [review]:

Thanks for the patch, Saurav. The CSS looks fine now.

::: src/application.js
@@ +407,3 @@
         settings = new Gio.Settings({ schema: 'org.gnome.documents' });
 
+        let defaultSettings = Gtk.Settings.get_default();

What about calling it 'gtkSettings'? We already have a 'settings', so it would make it clear what we are talking about here.

@@ +408,3 @@
 
+        let defaultSettings = Gtk.Settings.get_default();
+        this._themeChanged.provider = null;

I think the idiomatic way to do this in JavaScript is to define a global called cssProvider at the top of the file. Just like sourceManager or changeMonitor.

@@ +411,3 @@
+        defaultSettings.connect('notify::gtk-theme-name', Lang.bind(this, function() {
+            this._themeChanged(defaultSettings);
+        }));

What about simplifying this to:
gtkSettings.connect('notify::gtk-theme-name', Lang.bind(this, this._themeChanged));

Review of attachment 281611 [details] [review]:

::: src/application.js
@@ +392,3 @@
+        }
+        else if (this._themeChanged.provider != null)
+            Gtk.StyleContext.remove_provider_for_screen(screen, this._themeChanged.provider);

We also need to set cssProvider to null

(In reply to comment #8)
> Review of attachment 281611 [details] [review]:
> 
> ::: src/application.js
> @@ +392,3 @@
> +        }
> +        else if (this._themeChanged.provider != null)
> +            Gtk.StyleContext.remove_provider_for_screen(screen,
> this._themeChanged.provider);
> 
> We also need to set cssProvider to null

Or, maybe, not. Sorry about that.

Created attachment 281933 [details] [review]
Add application-specific theme extensions

Thanks for helping me out, Debarshi.