Page MenuHomePhabricator

Show a useful error message when notification popup opens while the user is logged out
Closed, ResolvedPublic

Description

  1. Log in, and open two tabs
  2. In tab 1, open the alerts popup; notifications are displayed
  3. In tab 2, log out
  4. Back in tab 1, open the messages popup.

A cryptic error is displayed:

Could not retrieve notifications. Please try again. (Error [object Object])

We should show a more comprehensible error message here.

Event Timeline

Catrope created this task.Dec 18 2015, 10:44 PM
Catrope raised the priority of this task from to High.
Catrope updated the task description. (Show Details)
Catrope added subscribers: Catrope, Mooeypoo.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 18 2015, 10:44 PM

For clarification: there are actually two bugs that are described here: [object Object] appearing instead of a legible API error and the fact we need a comprehensive message for logged out users. Fixing both.

Mooeypoo claimed this task.Dec 21 2015, 9:35 PM

Change 260495 had a related patch set uploaded (by Mooeypoo):
Display readable API error message

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

@Pginer-WMF and @jmatazzoni, the current commit fixes the described issue -- if a user is logged out before they opened their popup, it will display a helpful message saying they should login.

However, this will not do anything (currently by design) if the user already have notifications populated and is reopening the popup. In that case, nothing will happen -- the notifications will not be updated, but also the popup will not be empties. This was done so that the user can still click their notifications and go to wherever they lead even if their session is lost.

However #2, that means that "mark as read" and all the other operations that require logging in are ALSO not working, and we currently don't have any user-facing messages or indicators that tell the user something's wrong.

So what to do in those cases? Do we want to have an extra "fake" notification at the top if the user reopens the popup and is not logged in? In that case, should we also hide all the action "mark as read" buttons?

I'm leaving this for your consideration and design!

I'd like to help but not sure a) about button styles and b) what exactly the error message is meant to accomplish. Would it be along these lines"

Please log in to your [Wikipedia?] account?

[Login Link]
[Cancel]

(Parenthetically, I'm sure there is a style guide for erorr messages? If someone could point me to that I'd be grateful. Thanks.)

I know the message was already there and not changed as part of this ticket/patch but I just saw it for the first time.

While technically correct, I think the message (You must login to see your notifications.) is not too inviting. I could be a little bit friendlier and include a login link as a clear path forward. Maybe something like Please log in to see your notifications. <constructive login link here>

It not a big deal I'm just mentioning it as a suggestion.

Yeah, I tend to agree with you both, but I think that if we do that, we should change the current message for consistency.

The reason I went with the current message is because I saw that was the one being used when/if the user is not logged-in in Special:Notifications. It's more or less the same idea as the popup, so it should probably be the same for consistency.

I agree it should change, but I think we should change the message so that both use cases have the "nicer" phrasing and login link.

I like SBisson's version.

Please log in to see your notifications. <constructive login link here>

Pginer-WMF added a comment.EditedDec 22 2015, 5:23 PM

I agree with @SBisson, I'd prefer a positive message.
I'd suggest turning the background of the item to be light grey (#F1F1F1, as read items) to make it look like an empty panel with a message in it, not like a notification. Also, the item would lead to the log-in page when clicked (I used bold to emphasise this instead of using a link).

I adjusted the styles to simulate this and took a screenshot:

We could add a grey version of the logged-out icon we use on Flow (T110086#1591975) for consistency, but that may not be a high priority considering this is an edge case.

Perfect, I'll work on that.

My initial plan to unify the messages failed; the original message I saw (and used in this commit) is the message that appears at the top of the login screen, and it can't have parameters. It also makes no sense to have a link to the login because of that.

I've created a separate message, but adding a link internally within the message also proved a bit of an issue, because notification items are now designed to not have clickable text links -- so, incidentally, the design that's the easiest to implement is the design @Pginer-WMF shows in the screenshot, where the text is plain but the notification itself is clickable, going to the login screen.

Working on that now for this commit.

Notes for testing (after the fix) - different browsers tests:

  • Login in to two different browser windows
  • In one browser view Notifications (click on the icons) and log out
  • In another browser click on the Notification icons

I like SBisson's version.
Please log in to see your notifications. <constructive login link here>

If we have a colored link, it should be progressive, not constructive, because they're not instantly logged in when they click it.

The patch is ready for re-review according to @Pginer-WMF's design.

(Parenthetically, I'm sure there is a style guide for erorr messages? If someone could point me to that I'd be grateful. Thanks.)

The UI-Standardization is working on design guidelines. As far as I know, presenting errors and the tone of their copy has not been covered yet. @violetto and @Volker_E may have something in mind for upcoming quarters.

Change 260495 merged by jenkins-bot:
Display readable API error message

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

(Parenthetically, I'm sure there is a style guide for erorr messages? If someone could point me to that I'd be grateful. Thanks.)

The UI-Standardization is working on design guidelines. As far as I know, presenting errors and the tone of their copy has not been covered yet. @violetto and @Volker_E may have something in mind for upcoming quarters.

cc @jeffelder @Jbarbara

Checked in betalabs.

  1. The user warning "Please login to view your notifications" is displayed along with unchanged link 'Log out' indicating that a user is still logged in?

  1. For such cases - a user logs out, but still 'logged in' in different browser/tab - a user won't have normal warnings about IP address being displayed when he attempts to edit Flow pages. Non-Flow pages properly display the warning.
  1. Attempting to thank - 'Thank action' failed' is displayed.

Checked in betalabs.

Thanks.

  1. The user warning "Please login to view your notifications" is displayed along with unchanged link 'Log out' indicating that a user is still logged in?

Yes, the top-left link is controlled by core and is only updated at page load. So if they log out in another tab, or their session expires, it will not be updated until they reload.

  1. For such cases - a user logs out, but still 'logged in' in different browser/tab - a user won't have normal warnings about IP address being displayed when he attempts to edit Flow pages. Non-Flow pages properly display the warning.

T60696: Flow: It's too easy to accidentally edit when logged-out

Catrope closed this task as Resolved.Jan 19 2016, 2:40 AM