Page MenuHomePhabricator

Build Watchlist diff view
Closed, ResolvedPublic

Description

👨‍🎨 Check out the latest designs for this task on Zeplin.


1. Diff view2. Overflow menu3. Unwatch4. User profile (web view)5. User contributions (native, optional)6. Thanks: Dialog (shown once)7. Thanks: Snackbar8. Thanks: Already thanked
  1. Shows the main view of the screen.
  2. Show the overflow menu of the screen.
  3. State when tapping the 'WATCHING' button: Snackbar appears and button changes 'WATCH'.
  4. User profile (web view, preferably in-app) — Talk link leads users to native talk page
  5. Optional: User contributions page (reuses Watchlist main screen design)
  6. Thanks flow #1: Dialog appears after tapping the 'Thanks' button to raise awareness (only shown once)
  7. Thanks flow #2: Snackbar confirms that the 'Thanks' has been sent. Thanks button is greyed out.
  8. Thanks flow #3: Shows the Snackbar when 'Thanks' has already been sent for a specific edit.

The view consists of:

  • App bar
    • Back button
    • More menu
      • Share edit (icon) → triggers native share dialog with a link to this diff
      • %userName’s talk (icon) → takes users to the in-app talk page of this user
      • %userName’s contributions (icon) → takes users to the user contributions screen (external link)
  • Link to Watchlist item (article, talk, other)
  • Button
    • Watching
      • Full star icon when permanent
      • Half star icon when impermanent (→ Watchlist Expiry)
  • Date / time
  • Previous / next navigation buttons → to navigate back and forth between the Watchlist item’s edit history
  • Link to user profile (preferably in-app)
    • User icon (person or anonymous)
    • User name
  • Thank button
  • Indication of characters added / removed
  • Edit summary
  • Edit diff

Diff style / colors

  • Images below derive from iOS, we keep it consistent
  • Use Roboto Bold to highlight changes in diff
  • Use strike through to indicate removed characters
  • Background-color: → see table below
LightSepiaDarkBlack
Addition
font-color (color_group_64)base10green30green50green50
background-color (color_group_65)green90nonenonenone
Removal
font-color (color_group_66)base10red30red75red75
background-color (color_group_67)red90nonenonenone

More on colors: Visual Style: Colors – Wikimedia Design Style Guide

Event Timeline

Dbrant created this task.Dec 4 2020, 1:48 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 4 2020, 1:48 PM
LGoto triaged this task as Medium priority.Dec 7 2020, 5:03 PM

Hey @Sharvaniharan — I added a new color group 62, which basically consists of osage only ;) let’s try it, I might tweak it a bit after seeing the result :) thx!

Thank you @schoenbaechler

One more use case where the username is a huge anon address like this:

How should we handle this pushing the ui out of screen on the right?

schoenbaechler added a comment.EditedDec 11 2020, 9:54 AM

@Sharvaniharan can you float the thank button and char info to a new line when that happens? see a similar case in T269783. thanks!

@schoenbaechler asking the same questions from our slack chat here too, just for reference:
here.. https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655860f5a5c9dbf26c455
It is not possible to deep link an edit diff for the ‘Share Edit’ in the menu, so we might have to take it out.

here https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655837c345ea57e72a98d for the thank button, there is no way to determine if that user has thanked that edit before… it is not available on any apis… only the website knows this but not exposed to us… even on iOS they do it such that they gray the button as soon as the user thanks that edit, and once the edits are scrolled back and forth, that info is lost, and they can re-thank again. Since we don't get any error while re-thanking, I think we can do that as well. I feel even the graying of the button doesnt make too much sense.

And just confirming, the user name button click should send us to that user’s talk page? - makes the best sense.

Adding my replies from Slack for the sake of completion / source of truth:

@schoenbaechler asking the same questions from our slack chat here too, just for reference:
here.. https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655860f5a5c9dbf26c455
It is not possible to deep link an edit diff for the ‘Share Edit’ in the menu, so we might have to take it out.

Hmm, iOS is doing it. This link has been shared via iOS app. Per @Sharvaniharan’s answer, we’ll have to discuss this today (Dec 16).


here https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655837c345ea57e72a98d for the thank button, there is no way to determine if that user has thanked that edit before… it is not available on any apis… only the website knows this but not exposed to us… even on iOS they do it such that they gray the button as soon as the user thanks that edit, and once the edits are scrolled back and forth, that info is lost, and they can re-thank again. Since we don't get any error while re-thanking, I think we can do that as well. I feel even the graying of the button doesnt make too much sense.

Ok, I suggest to still grey it out once thanked when users are on the screen. If users go back, then it’s fine to display the button’s default state again. Greying out just prevents users from tapping this button multiple times in a row when they’re still on that screen. Per @Sharvaniharan’s comment, she’s going to implemented it that way.


And just confirming, the user name button click should send us to that user’s talk page? - makes the best sense.

No, it should lead users to a (in-app) profile page, per the task’s description:

4. User profile (web view, preferably in-app) — Talk link leads users to native talk page

https://zpl.io/VkgmGnK

Sharvaniharan added a comment.EditedDec 17 2020, 2:34 AM

@schoenbaechler could I please get the icons for profile and contributions options in the menu here : https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655860f5a5c9dbf26c455

@schoenbaechler could I please get the icons for profile and contributions options in the menu here : https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655860f5a5c9dbf26c455

... done (you should be able to export it from zeplin now)

@schoenbaechler Thank you for the icons. I have put them in. I think we should create 4 more color groups in the theme, for the edit add and removal font-colors and background colors.
Also a couple questions about the diff screen: https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655837c345ea57e72a98d

  1. is the bold diff text the comment on the edit?
  2. if a paragraph has been moved, how do we style it? We get a 'from' diff and a 'to' diff with the same text of the paragraph that was moved, but we do not get a highlight range [we can discuss this on a call to be more clear]

@schoenbaechler Thank you for the icons. I have put them in. I think we should create 4 more color groups in the theme, for the edit add and removal font-colors and background colors.

Yes, will do!

Also a couple questions about the diff screen: https://app.zeplin.io/project/57a120b91998d8977642a238/screen/5fc655837c345ea57e72a98d

  1. is the bold diff text the comment on the edit?

When you say comment do you mean edit summary? If so, yes exactly!

  1. if a paragraph has been moved, how do we style it? We get a 'from' diff and a 'to' diff with the same text of the paragraph that was moved, but we do not get a highlight range [we can discuss this on a call to be more clear]

If it has been moved, then it’s a removal (origin, red) and an addition (destination, green) in the diff view. Does that help? Happy to discuss this!

Thank you @schoenbaechler that works for paragraph switch. Also, correct edit summary.

@schoenbaechler Thank you for the icons. I have put them in. I think we should create 4 more color groups in the theme, for the edit add and removal font-colors and background colors.

Yes, will do!

I created the new color groups @Sharvaniharan, it’s color_group_64color_group_67. Will also add it to the task‘s description 👍.

Aren't Previous / next navigation buttons in the opposite position? Il you press the Left button you go to the next edit, and if you press the Right button you go to the previous edit. I don't know if this is intentional, but in the mobile web view the buttons are reversed (Left=Previous - Right=Next).

1.
2.
  1. This is the last edit on the app (can't press the left button because you're at the end)
  2. This is the same edit on mobile (There is no right button because you're at the end)

Thanks @Sharvaniharan for the work on this! Looks good so far, but there are a few things to address:

01) Agreeing with @YacineBoussoufa’s comment, the functionality of the arrows should be flipped.

Aren't Previous / next navigation buttons in the opposite position? Il you press the Left button you go to the next edit, and if you press the Right button you go to the previous edit. I don't know if this is intentional, but in the mobile web view the buttons are reversed (Left=Previous - Right=Next).

02) Can we include swipe left and right gestures to navigate between the edits?

03) Please align the set of arrows to the right

DesignvsImplementation

04) Text is a bit too close to the button, please add a 16dp margin between text and button.

05) Profile icon is out of proportion, please use Material’s person icon:

06) Overflow dialog has not been adapted for the different themes and is missing icons in dark and black mode.

SepiaDarkBlack

07)

a) Optimize transition between screens, see video below. Can we preload as much info as possible on this screen to avoid jumps? Maybe @cooltey can consult, he has managed to implement a great transition to the contribution detail screen (T269863)

b) Also, the thank heart appears to load in white (instead of blue) when accessing this screen in dark mode.

08) I realized the WATCHING button had an insufficient color contrast ratio in light and sepia theme. Please change it to color_group_68 from the guidelines

09) Can we respect the user’s setting for opening new links here? E.g. when page previews are enabled, display the page preview at the bottom.

If not possible, always display the page preview here instead of taking users to a new page.

I found also this issues:
If you open an edit of a page deleted it shows a generic error.

If you open an edit that protects a page it shows the error "No revision was found for parameter "rvstartid" ".

And you should't be able to see the 'thank' button when viewing your own edits.

@schoenbaechler I have addressed all comments but for 02) swiping, which will be too complicated technically. I also addressed

And you should't be able to see the 'thank' button when viewing your own edits.

Great work @Sharvaniharan — we’re at 95% 👏👏👏


01) Were you able to look into the issues that @YacineBoussoufa described (besides the thank button) @Sharvaniharan ? Because I experienced the same issue when wanting to access this article that has been moved (to a subcategory of German Wiki), see video below:

https://www.dropbox.com/s/ehos43u2nhiunkc/20210127_100558.mp4?dl=0

I found also this issues:
If you open an edit of a page deleted it shows a generic error.

If you open an edit that protects a page it shows the error "No revision was found for parameter "rvstartid" ".


02) Re: transitions

I have addressed all comments but for 02) swiping, which will be too complicated technically. I also addressed

How complicated is it? How many extra days would it take to implement it? It’s an essential aspect of the experience. Thanks for an estimate, I’ll check in with @JTannerWMF to see if we can include it then.

Even without supporting swipe gestures, we need to optimize the transition between edits. When navigating through article edits, the header (article title and WATCHING button) reloads every time. A complete reloading the anchoring element has the effect of loosing orientation. To improve the user’s experience, we need to make sure that the header stays when swiping through edits. It does not mean that the header needs to be sticky, it should just be consistent across screens.

@Dbrant @Sharvaniharan @cooltey

Per our discussion yesterday, I created a version that consider various Watchlist item states/cases.

The idea is, that we’re using the existing character button (e.g. + 494) and turn it into a disabled state button, when an item’s been moved, protected or deleted. Since the card is not tappable in these cases, I thought this is appropriate. Here’s the design:

**👉https://zpl.io/2jgwMY6**

Let me know what you think!

@Sharvaniharan

01) Just noticed that in certain cases the character count of when moving back and forth in the detail view does not update and the date seems to be formatted in an odd way? M01 28, 2021 | 17:00, see video below:

https://www.dropbox.com/s/6qi68qou8unuso1/20210128_171132.mp4?dl=0

02) Also, the WATCHING indicator should represent if an item’s been watched impermanently. Means, it should use the half star icon that’s used in the article view

02) Also, the WATCHING indicator should represent if an item’s been watched impermanently. Means, it should use the half star icon that’s used in the article view

AFAIK we don't get that information from the API (yet), so this will need to be done in a future iteration.

@Sharvaniharan

01) Were you able to look into the issues that @YacineBoussoufa described (besides the thank button) @Sharvaniharan ? Because I experienced the same issue when wanting to access this article that has been moved (to a subcategory of German Wiki), see video below:

https://www.dropbox.com/s/ehos43u2nhiunkc/20210127_100558.mp4?dl=0

I found also this issues:
If you open an edit of a page deleted it shows a generic error.

If you open an edit that protects a page it shows the error "No revision was found for parameter "rvstartid" ".

01) It seems like these cases are not completely addressed yet? See video below: https://www.dropbox.com/s/q5xq7nu5bb6nb2m/20210201_191533.mp4?dl=0

02 The new icons to indicate the MOVED, PROTECTED & DELETED state appear in weird cases in combination with character diff counts, see images below:

  • These states are solely intended to appear in combination with a MOVED, PROTECTED & DELETED label.
  • The cards should have reduced opacity when the states are used to indicate that users can’t tap on it, see designs on Zeplin.

Let me know if you have any further questions in regards to this...

@schoenbaechler I have fixed the UI issues, but for 01), since the page is deleted, user is seeing the error message in the video. Handling this in a more graceful way would have to be a v2 issue since we will also need to check if we get any feedback from the api in such cases and what message to show

I have also made a tiny change... since there is no profile page for anonymous users, i have made changes to hide this menu option when it is an anonymous user. Please test this too and lmk.

Ok, thx @Sharvaniharan. There are still issues left to be addressed 👇

01)

I have fixed the UI issues, but for 01), since the page is deleted, user is seeing the error message in the video. Handling this in a more graceful way would have to be a v2 issue since we will also need to check if we get any feedback from the api in such cases and what message to show

Yeah, I’d say not making it not tappable (disabled card) in the first place is the way to go for this. As far as I can tell, you’ve implemented it like this now?

02) I observed that button width differs even though they use the same amount of characters. Please unify the width to a value between +1 and +5 in the screenshot below:

03) Remove + from neutral edits (instead of +0 show 0)

04) Still seing M02 or M01 instead of the proper month in the detail view, please fix

@schoenbaechler Thank you for reviewing.. I will address the ui issues...

Ok, thx @Sharvaniharan. There are still issues left to be addressed 👇

01)

Yeah, I’d say not making it not tappable (disabled card) in the first place is the way to go for this. As far as I can tell, you’ve implemented it like this now?

A little unclear on this.. So as we discussed on our call the last time, If the edit itself is not a regular edit [Moved, deleted, protected] that edit item will NOT be clickable. However, If it is a regular edit but the page is deleted, it will be clickable but will result in an error feedback shown to the user... right?

Sharvaniharan added a comment.EditedTue, Feb 2, 2:32 PM

02), 03) and 04) Changes are done @schoenbaechler . Please test and Let me know.

@Sharvaniharan

A little unclear on this.. So as we discussed on our call the last time, If the edit itself is not a regular edit [Moved, deleted, protected] that edit item will NOT be clickable. However, If it is a regular edit but the page is deleted, it will be clickable but will result in an error feedback shown to the user... right?

Okay got it → I created a new task to address it specifically.


02), 03) and 04) Changes are done @schoenbaechler . Please test and Let me know.

Looks good to me! Moving it to QA signoff! 🎉

Thank you @schoenbaechler !! 🎉🎉🎉

@Dbrant - Testing on 2.7.50341-alpha-2021-02-02 and I noticed a couple of odd things. I'm not sure if they block this bug or maybe new items:

Thanks -
Thanking seems to work and once an edit has been thanked it shows the grayed out 'thanks' button, but tapping it again does not produce the already thanked snackbar.

After thanking on an edit, if the user navigates back to the watchlist and then taps the same edit again, then user can send another thanks. I think it should remain grayed out.

User contributions -
Tap on an edit and tap on the overflow menu. Selecting the user contribution shows the app chooser and selecting Wikipedia shows the app chooser again putting the user into a loop.

Dbrant added a comment.Tue, Feb 2, 7:40 PM

Thanks @ABorbaWMF
These are all basically known issues, and/or things that are limitations of the Thanks API. I'll make a new task to capture the work of persisting edits that have been thanked (which we don't get from the API).

@ABorbaWMF Thank you for testing this.

For the thank button, the greying out and when moving back and forth being able to thank again is expected behavior since there are api shortcomings... per this discussion: https://phabricator.wikimedia.org/T269447#6694860
Sorry for not updating the task description. I am assuming we removed the requirement for another snackbar when the grayed out thank button is clicked, and instead opted to make it unclickable... @schoenbaechler lmk if that is correct.

User contributions chooser is an actual issue which I think we need a long term solution but is not a blocker for this since it happens in other places of the app too, and it was a trade-off for a bug solved on Samsung devices CC @Dbrant

Dbrant closed this task as Resolved.Tue, Feb 2, 10:04 PM