Page MenuHomePhabricator

Watchlist Expiry: Spinning star should be filled when permanent > temporary [[SMALL]]
Closed, ResolvedPublic

Description

As a Watchlist Expiry user, I want the star to be fully shaded when it transitions from permanent to temporary (rather than blank), so the transitional state is more clear to me as a user.

Background: Right now, if I watch a page permanently, the star spins & goes from blank to fully shaded. If I unwatch it, it spins and goes from fully shaded to blank. However, if I go from permanent to temporary, the star spins and goes from blank to half shaded. This is incorrect behavior; it should go from fully shaded to half shaded.

Acceptance Criteria:

  • When temporarily watching a page from dropdown, the spinning star should be fully filed during the spin process, before becoming half shaded
  • When temporarily watching a page from dropdown, the watch link should say watching during the spin process when there is no star.

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptJul 28 2020, 5:15 PM
ifried updated the task description. (Show Details)
ARamirez_WMF renamed this task from Watchlist Expiry: Spinning star should be filled when permanent > temporary to Watchlist Expiry: Spinning star should be filled when permanent > temporary [[SMALL]].Jul 30 2020, 5:28 PM
ARamirez_WMF moved this task from To Be Estimated/Discussed to Estimated on the Community-Tech board.

Change 618622 had a related patch set uploaded (by HMonroy; owner: HMonroy):
[mediawiki/core@master] Update spinning star when temporarily watching an article

https://gerrit.wikimedia.org/r/618622

Change 619844 had a related patch set uploaded (by HMonroy; owner: HMonroy):
[mediawiki/skins/Vector@master] Add class to spin full star when watching temporarily

https://gerrit.wikimedia.org/r/619844

Change 622441 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/core@master] Watchlist: Fix updateWatchLink removing css class when action=watch

https://gerrit.wikimedia.org/r/622441

Change 618622 abandoned by HMonroy:
[mediawiki/core@master] Update spinning star when temporarily watching an article

Reason:
This change is covered by https://gerrit.wikimedia.org/r/c/mediawiki/core/ /622441

https://gerrit.wikimedia.org/r/618622

To easily explain what I mean I'll be referencing each star with a number representing their position in the image below with the first star being #1 and the last one being #7

Before the watchlist expiry was introduced #7 was the "loading" star regardless if the page is in your watchlist or not. You can currently see it in production if you click watch or unwatch and then click elsewhere so to remove the focus on the star; it will always switch to #7.

@Prtksxna @ifried Based on the acceptance criteria on this task, is there a reason to keep #7 around? Can't we just spin the current image and then switch to the new state? #7 is almost never visible and the rotating star already indicates that you are waiting for an action.

Here is what I mean, for example: (if the star is focused or hovered the equivalent star with the golden border will spin)

transitionspining star
not watching -> watch#5 or #6
perm watch -> temp watch#1 or #2
temp watch -> unwatch#3 or #4

As a bonus, if we get rid of #7 it will simplify a lot the css rules.

watch-stars.png (239ร—1 px, 60 KB)

Thanks for sharing this very detailed and useful explanation, @dmaza!

I have no product objections with removing star #7, since a) you can barely see #7 when switching between watch/unwatch on production (so I don't see an impact for users), b) I don't think #7 communicates something unique/necessary to users (since the transition is so fast, c) this solution would simplify the existing engineering work, and d) the work to fix the star state when switching from permanent > temporary is more important than the potential preservation of #7 (since it is much more confusing UX, in my view), if we're thinking about priority/trade-offs.

However, I would like to know what @Prtksxna thinks and if he agrees first. I may be overlooking something. Also, I do not know if it is okay to simply remove an existing state for the star or if any larger design review/approval process would be expected (I'm curious to hear what Prateek says about this question, as well). In that case, I'll await Prateek's response. Thanks!

Yep, getting rid of #7 makes sense. I mostly see #2 or #6 spinning any way. The only way to see #7 (for me on Chrome on MacOS) is to click the star so that it starts spinning and click somewhere else so that it loses focus.

@dmaza the table you shared looks good too. Thank you so much for this explanation ๐Ÿ™๐Ÿฝ

I have removed the background image from loading state in patch https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/619844. The current background image will spin when transitioning between watches.

Thank you all!!

Change 619844 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Remove the loading background image for loading class

https://gerrit.wikimedia.org/r/619844

Change 622441 merged by jenkins-bot:
[mediawiki/core@master] Watchlist: Fix updateWatchLink removing css class when action=watch

https://gerrit.wikimedia.org/r/622441

dom_walden added a subscriber: dom_walden.

Tested all transitions from one watched state to another:

  • perm -> temp: full spinning star (bug from description)
  • unwatched -> perm: empty spinning star
  • perm -> unwatched: full spinning star
  • temp -> unwatched: half spinning star
  • temp -> temp: half spinning star
  • temp -> perm: half spinning star

Whether or not the watch star has focus (as described in T259053#6419701) does not seem to make a difference to how it looks.

  • When temporarily watching a page from dropdown, the watch link should say watching during the spin process when there is no star.

This is true for Modern and MonoBook.

I tested watch star behaviour on all the skins and mobile. It seemed to behave fine. Except obviously the transition animation is different on different skins, and Minerva and Mobile don't have a half watch star.

I also disabled watchlist expiry and retested watch star on all the skins. Seemed fine as well.

Note for testing: If you are struggling to see the transition because it is too quick, most browsers allow you to artificially throttle your connection via the developer tools (normally available pressing F12):

Test environments:

I have tested this on https://meta.wikimedia.beta.wmflabs.org/ and it looks good. As Dom noted, the transition between various states of watched permanently, watched temporarily, and unwatched look good, and there is no longer the display of the empty star when transitioning between permanent and temporary watch status. For this reason, I'm marking this work as Done.

Change 626529 had a related patch set uploaded (by Dmaza; owner: HMonroy):
[mediawiki/skins/Vector@REL1_35] Remove the loading background image for loading class

https://gerrit.wikimedia.org/r/626529

Change 626530 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/core@REL1_35] Watchlist: Fix updateWatchLink removing css class when action=watch

https://gerrit.wikimedia.org/r/626530

Change 626529 merged by jenkins-bot:
[mediawiki/skins/Vector@REL1_35] Remove the loading background image for loading class

https://gerrit.wikimedia.org/r/626529

Change 626530 merged by jenkins-bot:
[mediawiki/core@REL1_35] Watchlist: Fix updateWatchLink removing css class when action=watch

https://gerrit.wikimedia.org/r/626530