890225 - [Notifications] »Clear all« and header shows even with no notifications

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Jan-Christoph Borchardt]

User Agent: Mozilla/5.0 (X11; Linux armv7l) AppleWebKit/536.11 (KHTML, like Gecko) Ubuntu/12.04 Chromium/20.0.1132.47 Chrome/20.0.1132.47 Safari/536.11

Steps to reproduce:

Have no notifications and bring down the notification tray.


Actual results:

The »Notifications« header is there, and »Clear all« is displayed on the right. Albeit greyed out, it’s a bit confusing and seems like there should be notifications there.


Expected results:

Instead of displaying the header like that with the »clear all« on the right, it should probably be »No notifications« or something similar. As feedback that there’s nothing there.

Comment 1

[Tom Leese]

Attachment: pull request

I followed the instructions on https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Platform/Gaia/Hacking#Submitting_a_patch to fix this bug. I wasn't sure about how to file or assign myself to the bug though, so I just fixed it and have submitted the pull request.

Sorry if I have got this wrong somehow!

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 817298 [details] [review]
pull request

This is an excellent patch and thank you for patching Gaia.
Besides the comment on Github, you would also need to patch unit tests and make sure Travis-CI runs green:

https://travis-ci.org/mozilla-b2g/gaia/jobs/12581972#L1522

I will flag UX in the next comment to see if this is the desired interaction.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Hi UX team, [e-mail] here made a patch so what when there no notification, The "Notifications - clear all" header will be replaced with "No notifications". Is that the desired interaction? Or should we simply hide the "clear all" button, or the header itself?

I will help [e-mail] to come up another patch when the interaction is decided.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #3)
> Hi UX team, [e-mail] here made a patch so what when there no
> notification, The "Notifications - clear all" header will be replaced with
> "No notifications". Is that the desired interaction? Or should we simply
> hide the "clear all" button, or the header itself?

Sorry, that's not exactly what his patch did. His patch would make the header show "Notifications - No notifications" if I didn't misread it.

Comment 5

[Tom Leese]

Comment on attachment 817298 [details] [review]
pull request

I think I've fixed all the issues noted (except for the UX one) in the original patch.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 817298 [details] [review]
pull request

Wonderful, thanks.

I am setting ui-review? so that people won't land it prematurely.

Comment 7

[Stephany Wilkes]

Flagging Rob for a Sys FE check of this patch, especially of how it shows "No notifications."

Comment 8

[Jacqueline Savory [:jsavory] UX]

Yes, the header should change from "Notification" to "No notifications" when the user has no items in the utility tray.

However, the "Clear All" button should remain where in place and instead should grey out to display that it is no longer actionable to users.

Comment 9

[Tom Leese]

Comment on attachment 817298 [details] [review]
pull request

I've changed how my patch works to follow the UX guidelines as outlined above.

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 817298 [details] [review]
pull request

https://travis-ci.org/mozilla-b2g/gaia/jobs/13204733#L1540

There is a test failure... could you fix it?

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Do remember to reset the review? flag to me please.

Comment 12

[Tom Leese]

Comment on attachment 817298 [details] [review]
pull request

Fixed the issue with the tests, passes in Travis CI now.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

master: https://github.com/mozilla-b2g/gaia/commit/04c1e38c5186cb06f6e61c4b80ab1aff3d4e3ae9

Thank you again for your contribution!