Created attachment 272400 [details] [review]
dateMenu: Refactor function

Refactor function called on selected-date-changed signal
since we will need to do more things in there in an upcomming
patch.

Created attachment 272401 [details] [review]
dateMenu: Don't make dateMenuButton reactive on current day

Having the ability to go to the current date if the user is already
in the current date can be confusing. So don't make the button reactive
until the selected date change.

Review of attachment 272400 [details] [review]:

Not sure that's necessary for 1-2 additional lines; in any case it should be squashed with the main patch.

Review of attachment 272401 [details] [review]:

::: js/ui/dateMenu.js
@@ +75,3 @@
+        // until the selected date change.
+        this._date.reactive = false;
+        this._date.track_hover = false;

Why do you need to change track_hover?

@@ +146,3 @@
+    },
+
+    _diffDays: function(date1, date2) {

You are actually only interested in whether a given date is today or not, so why not use something like:
_isToday(date) {
    let now = new Date();
    return now.getYear() == date.getYear() &&
              now.getMonth() == date.getMonth() &&
              now.getDay() == date.getDay();
}

Review of attachment 272401 [details] [review]:

::: js/ui/dateMenu.js
@@ +75,3 @@
+        // until the selected date change.
+        this._date.reactive = false;
+        this._date.track_hover = false;

If you say about in this specific line, just for clarity with the changes done after. If you think it doesn't do it more clear, I just erase it since it is not necesary.

If you say in general, when I change to reactive = false, if the user has hovered the button, the hover state is preserved. So that's why I change the track_hover.

@@ +146,3 @@
+    },
+
+    _diffDays: function(date1, date2) {

ok

(In reply to comment #3)
> Review of attachment 272400 [details] [review]:
> 
> Not sure that's necessary for 1-2 additional lines; in any case it should be
> squashed with the main patch.

Right, I was not sure what to do.

(In reply to comment #5)
> If you say in general, when I change to reactive = false, if the user has
> hovered the button, the hover state is preserved. So that's why I change the
> track_hover.

Mmh, but how would that happen?

Review of attachment 272401 [details] [review]:

::: js/ui/dateMenu.js
@@ +75,3 @@
+        // until the selected date change.
+        this._date.reactive = false;
+        this._date.track_hover = false;

Also, you should set those properties when constructing the object.

(In reply to comment #7)
> (In reply to comment #5)
> > If you say in general, when I change to reactive = false, if the user has
> > hovered the button, the hover state is preserved. So that's why I change the
> > track_hover.
> 
> Mmh, but how would that happen?

Go to another day than today, go to button (hovered now), click button, it changes to today, it becomes not reactive, hover state preserve.

Created attachment 272409 [details] [review]
dateMenu: Don't make dateMenuButton reactive on current day

Having the ability to go to the current date if the user is already
in the current date can be confusing. So don't make the button reactive
until the selected date change.

Review of attachment 272409 [details] [review]:

::: js/ui/dateMenu.js
@@ +66,3 @@
+        // Having the ability to go to the current date if the user is already
+        // in the current date can be confusing. So don't make the button reactive
+        // until the selected date change.

"in the current date" => "on the current date", "the selected date change" => "the selected date changes"

@@ +89,3 @@
+
+                                   // Make the button reactive only if the selected date is not the current date.
+                                   // It need to set track_hover accordingly, since when the date changes

"It needs"

@@ +92,3 @@
+                                   // because the button is pressed, the button becomes not reactive,
+                                   // and then the hover state is preserved.
+                                   this._date.track_hover = this._date.reactive = !this._isToday(date)

Messing around with StWidget:track-hover still feels messy. Is there any use case for freezing the hover state when the actor becomes unreactive? If not, then this should be handled this in StWidget itself IMHO.

Created attachment 274460 [details] [review]
dateMenu: Don't make dateMenuButton reactive on current day

Having the ability to go to the current date if the user is already
in the current date can be confusing. So don't make the button reactive
until the selected date change.

as we said, StWidget now disables hover state if not reactive.
https://bugzilla.gnome.org/show_bug.cgi?id=728343

Review of attachment 274460 [details] [review]:

The errors you fixed in the comment ("in the current date", "the selected date change") are still present in the commit message. Otherwise LGTM.

Comment on attachment 274460 [details] [review]
dateMenu: Don't make dateMenuButton reactive on current day

Attachment 274460 [details] pushed as 19afabe - dateMenu: Don't make dateMenuButton reactive on current day