ev_document_model_set_inverted_colors is what we want.

Created attachment 300776 [details] [review]
preview: implement dark-mode in the content

Review of attachment 300776 [details] [review]:

Thanks for the patch, Alessandro.

::: src/preview.js
@@ +548,3 @@
+
+            let settings = Application.settings;
+            settings.connect('changed::night-mode', Lang.bind(this,

We need to keep the connection ID so that we can disconnect the signal later when the view is destroyed because the Application will live longer than the view. Look for _viewSettingsId in src/mainToolbar.js for an example.

Also, you can use Application.settings directly instead of putting it in a variable.

@@ +550,3 @@
+            settings.connect('changed::night-mode', Lang.bind(this,
+                function () {
+                    state = settings.get_value('night-mode');

Don't you need a 'let' when using 'state' for the first time?

@@ +554,3 @@
+                }));
+            let state = settings.get_value('night-mode');
+            this._model.set_inverted_colors(state.get_boolean());

Better to wrap these two lines in a function, which you can connect to the signal and also call here. See src/mainToolbar.js for an example.

Review of attachment 300776 [details] [review]:

::: src/preview.js
@@ +554,3 @@
+                }));
+            let state = settings.get_value('night-mode');
+            this._model.set_inverted_colors(state.get_boolean());

Also remember that this._model can be null.

Created attachment 300840 [details] [review]
preview: implement dark-mode in the content

I have some trouble:
I'm not sure that we should keep the connection ID to disconnect it later. PreviewView seems to be never destroyed, instead is reset when necessary. There is also a connection to 'action-state-changed::present-current' that is never disconnected.
If we listen for the action will be activated when someone modify gsettings with: "gsettings set org.gnome.documents night-mode [true|false]"?
Currently it is listening for the action, but in the callback I can't call "get_state()" on the action parameter: it says that isn't a function, but in application.js is used. Any idea why?

(In reply to Alessandro Bono from comment #6)
> I'm not sure that we should keep the connection ID to disconnect it later.
> PreviewView seems to be never destroyed, instead is reset when necessary.

Why do you think it is never destroyed? Widgets are usually destroyed when their parents are destroyed. In this case PreviewView.widget gets destroyed when the top level window is closed. If you connect to the 'destroy' signal and you will see your handler getting called.

> There is also a connection to 'action-state-changed::present-current' that
> is never disconnected.

Yes, there are some cases like that. I think we should disconnect those too. Please use a separate patch if you do that.

> If we listen for the action will be activated when someone modify gsettings
> with: "gsettings set org.gnome.documents night-mode [true|false]"?

Yes, it should.

> Currently it is listening for the action, but in the callback I can't call
> "get_state()" on the action parameter: it says that isn't a function, but in
> application.js is used. Any idea why?

That is because in application.js, the handler is connected to GSimpleAction::activate, while you are listening to GActionGroup::action-state-changed. The parameters passed to those handlers are different. See _onPresentStateChanged for an example of what you want.

Created attachment 301238 [details] [review]
preview: implement dark-mode in the content

Review of attachment 301238 [details] [review]:

::: src/application.js
@@ +205,3 @@
             function() {
                 let state = settings.get_value('night-mode');
+                if (state.get_boolean() != action.state.get_boolean())

Could you please put this in a separate patch and note that it is a fall out from bd6ce176d055e23b8f4aaf0ed42c5fb542640aa3

::: src/preview.js
@@ +53,3 @@
         this._model = null;
         this._jobFind = null;
+        this._nightModeId = 0;

This doesn't need to be a instance variable. Nor does it need to be initialized.

@@ +150,3 @@
                                             Lang.bind(this, this._onLoadError));
+        this._nightModeId = Application.application.connect('action-state-changed::night-mode',
+                                            Lang.bind(this, this._updateNightMode));

Please move it above the present-current block, and the align the Lang.bind as is done for _onPresentStateChanged.

@@ +154,3 @@
+        this.widget.connect('destroy', Lang.bind(this,
+            function() {
+                if (this._nightModeId != 0) {

No need for this check, or setting it to 0 afterwards. It can't be 0 and the handler will be called only once.

@@ +563,3 @@
+
+    _updateNightMode: function(source, actionName, state) {
+        if(this._model) {

Missing white space after 'if'.

@@ +565,3 @@
+        if(this._model) {
+            if(state == null) {
+                state = Application.settings.get_value('night-mode');

Becomes simpler if you use:
let nightMode = Application.settings.get_boolean('night-mode');

@@ +570,1 @@
         }

I pushed a patch to make this even simpler. No need for accepting any arguments. Just directly read the setting. See _updateTypeForSettings for an example.

Created attachment 301307 [details] [review]
application: Simplify the night-mode state changes

Created attachment 301315 [details] [review]
application: Compare boolean values

Fall out from bd6ce176d055e23b8f4aaf0ed42c5fb542640aa3

Created attachment 301316 [details] [review]
preview: Implement dark-mode in the content

Review of attachment 301315 [details] [review]:

Perfect. Thanks, Alessandro.

Review of attachment 301316 [details] [review]:

Perfect. However, it needs to be rebased after the patches in bug 747506

Created attachment 301325 [details] [review]
preview: Implement dark-mode in the content

Review of attachment 301325 [details] [review]:

I attributed the patch to "Alessandro Bono <shadow@openaliasbox.org>" so that we have a full name and proper email address.