Page MenuHomePhabricator

Flag talk page feedback in toolbar
Closed, ResolvedPublic

Description

Value proposition

From original task description:

What often happens is that reviewers will put some partial effort into reviewing an article, and the give up after hitting a roadblock and being unsure. An ability to write a comment on the talk page that is automatically flagged by the toolbar if another reviewer loads the page would be ideal.

As part of T207443: Feedback for creator should also be posted to article talk page, the comment in the review field is already posted on the article talk page with a dedicated section heading (Feedback from New Page review process - template/configurable by community?). This task is for flagging the presence on feedback on the talk page in the toolbar.

Acceptance criteria:

  • If a reviewer lands on a page in the feed that has talk page feedback (as provided via T207443), the Info flyout should flag that -

.

    • Text: Previous reviewers of this article left comments on the talk page. (The words talk page link to the feedback section on the talk page).
  • The text does not show up if there is no prior feedback.

Note:

Details

Related Gerrit Patches:
mediawiki/extensions/PageTriage : masterPrevent all notification badges from obscuring clicks on toolbar icons
mediawiki/extensions/PageTriage : masterPrevent badges from getting in the way of clicks
mediawiki/extensions/PageTriage : masterToolbar notice about talkpage feedback

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Ping @Samwilson - wrong sam pinged before.

aezell added a subscriber: aezell.Jun 28 2019, 2:43 PM

@Samwilson I'm OK with repurposing it as it'll keep all the context in one place.

I'm okay with repurposing as well. Will bring it back for estimation.

Niharika renamed this task from [4 hours] Flag talk page feedback in toolbar to Flag talk page feedback in toolbar .Jul 1 2019, 4:42 PM
Niharika removed MusikAnimal as the assignee of this task.
Niharika moved this task from Untriaged to To be estimated/discussed on the Community-Tech board.
Niharika added a subscriber: MusikAnimal.
Niharika changed the subtype of this task from "Spike" to "Task".Jul 1 2019, 5:39 PM

Change 521445 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/PageTriage@master] Toolbar notice about talkpage feedback

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

JTannerWMF moved this task from Inbox to External on the Growth-Team board.Jul 9 2019, 7:22 PM
JTannerWMF added a subscriber: JTannerWMF.

Looks like Community-Tech is working on this

I think the styling of the info flyout has changed a bit since this issue was created. The above patch looks like the following; is it okay?

That looks fine @Samwilson but how does the toolbar indicate that a message has been left?

You mean, like the red number bubble that indicates possible issues? There's nothing like that at the moment.

Some indicator, ideally different from the red bubble with the number of issues, to indicate a talk page message is present would be incredibly useful with this - there aren't always reasons to open that particular panel and so a NPP note could get missed by another reviewer.

Sure thing. We could add an extra badge at the bottom corner, e.g.:

Would that be okay?

That would work well in my opinion. Thanks Sam.

Cool, done.

This is now ready for review. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/521445

Maybe the 2nd toolbar badge could be a different colour than red, to distinguish it? Or not. And I don't know about 'T' as the indicator; what should it be?

Could be a Mail icon . I personally don't like having two icons there. It looks cramped and confusing. Not sure what's a better way though.

Could the background of the i change (like the Check changing from grey to green when patrolled) for one of these? Perhaps make the issue change the i to red and then add an icon like @Niharika's mail icon for the message? The background color loses the number of issues present but that seems less important that alerting that there are issues in my thinking.

I think the second icon looks a bit weird, too. Would it make more sense to put this information on the "Mark as reviewed" button/flyout, which eventually will leave feedback on the talk page (T207443) ? That way communication-related stuff is all in one place.

We could also have the badge be a count of talkpage messages, if that's of any use.

Re moving it to the 'mark' flyout: sounds good to me, but there are some situations aren't there in which that icon/flyout isn't shown? Would it be annoying to not have the feedback info visible for those pages?

To maximize the use of this feature I would suggest some visual indicator on the toolbar itself - no clicks or flyouts necessary. I leave it to others to design the best way to do this.

@Prtksxna Your expert design advise is needed on this ticket.

I took @Samwilson's help to understand this navigation a bit better. I now understand that the icon currently represents two states:

What?TypeHow?
Open / closeBooleanBlue / Gray
Possible issuesNumberCount badge

We're proposing to add another —

Comments on the talk pageBoolean???

Is there a possibility of this being a count too?


By adding this information on the icon we might be overloading it with a lot of state. This might be more confusing than helpful (especially for people who don't use it on a regular basis). We should consider carefully if this is necessary. Some possible solutions:

  • Two icons: We could have the option of showing two dots of different colors (without any numbers inside them). We'd be representing the possible issues information as Boolean too in this case.
  • Color: (as suggested by @Barkeep49 above) Use the color of the icon (in the non selected state) to show that there are messages here. This is more ambiguous than the green checkmark because its not obvious what state a colored info icon could represent.
  • New icon: A new icon below the that has a count or a simple red dot badge (based on whether we can get a count of the messages or not)

Is there a possibility of this being a count too?

Yep, it is not too hard to get the count of talk page messages.

This is more ambiguous than the green checkmark because its not obvious what state a colored info icon could represent.

We could add a tooltip containing e.g. "There are 3 talk page messages"

So... who wants to make a decision here? :)

aezell added a subscriber: ifried.Jul 31 2019, 12:39 PM

Tagging @ifried to get her feedback.

ifried added a comment.EditedAug 1 2019, 6:11 PM

I prefer the third option outlined by @Prtksxna: A new mail icon below the info icon with a count.

Here's why: This option provides the most information to users, although I understand that it looks a bit cluttered. With the addition of a new icon, users can clearly see that there are messages to check out. By contrast, I think option #1 provides too little information and option #2 is too ambiguous. While we can add a tooltip, I would like to select a solution that doesn't require additional explanation to users. As for the counter, I think that providing more (rather than less) information is a better choice. For example, if there's a large number of talk page messages, I imagine that could be very helpful to know at a quick glance.

@Barkeep49, @Insertcleverphrasehere, and everyone else -- How do you feel about this option?

@ifried Works for me. Thanks for everyone's work on this element of the task.

ifried added a comment.Aug 5 2019, 8:47 PM

@Barkeep49 -- Thanks for your response!
@Samwilson -- Let's proceed with the following: A new mail icon below the info icon with a count. Thanks!

I've updated it to look like this:

Also changed the message to include the count: "Previous reviewers of this article have left (a comment|x comments) on the [talk page]."

Ready for re-review: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/521445

ifried added a comment.Aug 7 2019, 9:16 PM

Thanks, @Samwilson. The updated version (icon display & message) looks good to me.

Change 521445 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Toolbar notice about talkpage feedback

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

dom_walden added a subscriber: dom_walden.

When the article's Talk page has an edit(s) with the tag "pagetriage"[1] the mail icon appears in the toolbar, with a count.

I checked that the count was correct (by comparing to the Talk page's revision history).

Revisions that have been undone or hidden are still counted.

If the Talk page is deleted or moved (leaving redirect) then the mail icon no longer appears. I am not sure in what circumstances you would only move an article's Talk page (and not the article itself) and whether, in these circumstances, you would still want the moved Talk page's feedback to be flagged.

The mail icon does not appear for User pages (even if they have edits with the "pagetriage" tag). This makes sense as PageTriage does not support posting feedback for User pages.

I could see no aesthetic issues on Firefox, Chromium, Safari, IE11 and Edge 18.

The only possible UI issue is that clicking the mail icon does not open the "Page info" flyout. You have to click the "i" round button. The mail icon does cover up quite a lot of the button, especially if the count is in the double figures and when the button also has the "Possible issues" count in the top right. See, for example, https://en.wikipedia.beta.wmflabs.org/wiki/Drwpb_Page_3 and below screenshot.

Notes

  1. This currently isn't done by the tool itself (until T207443), but can be simulated via the API or running this JS code in the browser
ifried added a comment.EditedAug 13 2019, 3:54 PM

Thanks for the thorough QA testing, @dom_walden. Also, great point about the possible UI issue (i.e. clicking the mail icon doesn't open the "Page info" flyout). While I don't necessarily see this as a dealbreaker (since the user isn't blocked from accessing the flyout), the combination of blocking so much space & not having any direct functionality (other than informing the user of Talk page updates) is a bit awkward. With that in mind, I have a quick question for you: Does clicking on the other icon (i.e. the count of potential issues) result in opening the "Page Info" flyout? If not, I think the lack of functionality for the lower Talk-related icons is probably fine, since they mirror the behavior of the above icons.

Also, @Prtksxna, what do you think? I know we have had some conversations about the current state of the icon representation, and I would be curious to know your thoughts. Do you think we need to perhaps move the mail + counter icons down a bit (so they block less space)? Thanks!

dom_walden added a comment.EditedAug 13 2019, 4:24 PM

...Does clicking on the other icon (i.e. the count of potential issues) result in opening the "Page Info" flyout?

No. Both icons currently behave the same.

ifried added a comment.EditedAug 20 2019, 7:59 PM

Update: We have decided to set the pointer-events to 'none.` This means that the click event will be applied to whatever is behind it (which, in this case, is the button itself -- and, thus, would open the fly-out).

Change 531592 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/PageTriage@master] Prevent badges from getting in the way of clicks

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

Change 531592 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Prevent badges from getting in the way of clicks

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

Testing on Firefox 60, Safari 12 and IE11, Sam's latest patch looks fine. If you click on the part of the mail icon which overlaps the round "i" button will open the "Page Info" flyout.

Apparently the pointer-events property will not work for Internet Explorer older than 11. However, we do not appear to support those browsers (I am not sure where this is documented).

@Insertcleverphrasehere @Barkeep49 @DannyS712 This work should be on production. When you get a chance, can you confirm that you're seeing the expected changes? Thank you!

@Insertcleverphrasehere @Barkeep49 @DannyS712 This work should be on production. When you get a chance, can you confirm that you're seeing the expected changes? Thank you!

Nope: on the beta cluster, the talk page email icon does not get in the way, but the potential issues number at the top does. Below, I've copied the DOM in case that helps. I note that the number of issues in wrapped in a div, while the feedback icon is in a span - could that be the cause?

#mwe-pt-info > .mwe-pt-tool-icon-container
<div class="mwe-pt-tool-icon-container">
     <img class="mwe-pt-tool-icon" src="/w/extensions/PageTriage/modules/ext.pageTriage.views.toolbar/images/icons/normal/icon_info.png" width="35" height="35" title="Show metadata for this page">
     <div class="notification-badge notification-badge-top notification-badge-important">
          <span class="notification-badge-content">5</span>
     </div>
     <span class="mwe-pt-talkpage-feedback-badge">
          <div class="notification-badge notification-badge-bottom notification-badge-important oo-ui-iconElement oo-ui-icon-message oo-ui-image-invert" title="Previous reviewers of this article have left 11 comments on the talk page.">
               <span class="notification-badge-content">11</span>
          </div>
     </span>
</div>

Change 534715 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/PageTriage@master] Prevent all notification badges from obscuring clicks on toolbar icons

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

Sorry, I got the last patch wrong; it didn't handle all the badges. Fixed in the above patch.

Change 534715 merged by HMonroy:
[mediawiki/extensions/PageTriage@master] Prevent all notification badges from obscuring clicks on toolbar icons

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

Okay, the last patch (I shouldn't jinx it) is merged and this is ready for QA again.

Now both the badges for number of possible issues and the number of previous comments don't get in the way of the Page info icon. (This is testing on Beta. It does not appear to be on Production)

I believe that is the only two badges PageTriage uses.

@Barkeep49 @DannyS712

These changes should now be on production (which include the fix so that the badges don't block the Page Info icon). When you get a chance, can you confirm if you're seeing these changes? Thanks!

It seems to be working for me - took a little bit of time to come across an article with it.

ifried closed this task as Resolved.Sep 26 2019, 9:16 PM

Thanks for confirming, @Barkeep49!

I'll mark this work as Done.