Closed
Bug 890225
Opened 11 years ago
Closed 11 years ago
[Notifications] »Clear all« and header shows even with no notifications
Categories
(Firefox OS Graveyard :: Gaia, defect)
Firefox OS Graveyard
Gaia
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: hey, Assigned: leese.thomas81)
Details
(Whiteboard: [mentor=timdream])
Attachments
(1 file)
46 bytes,
text/x-github-pull-request
|
timdream
:
review+
timdream
:
ui-review+
timdream
:
feedback+
|
Details | Review |
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.
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!
Attachment #817298 -
Flags: review?
Attachment #817298 -
Flags: review? → review?(timdream)
Updated•11 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
Comment 2•11 years ago
|
||
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.
Attachment #817298 -
Flags: review?(timdream) → feedback+
Comment 3•11 years ago
|
||
Hi UX team, leese.thomas81@gmail.com 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 leese.thomas81@gmail.com to come up another patch when the interaction is decided.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 4•11 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #3) > Hi UX team, leese.thomas81@gmail.com 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 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.
Attachment #817298 -
Flags: review?(timdream)
Comment 6•11 years ago
|
||
Comment on attachment 817298 [details] [review] pull request Wonderful, thanks. I am setting ui-review? so that people won't land it prematurely.
Attachment #817298 -
Flags: ui-review?
Attachment #817298 -
Flags: review?(timdream)
Attachment #817298 -
Flags: review+
Updated•11 years ago
|
Assignee: nobody → leese.thomas81
Updated•11 years ago
|
Flags: needinfo?(rmacdonald)
Flags: needinfo?(jsavory)
Comment 7•11 years ago
|
||
Flagging Rob for a Sys FE check of this patch, especially of how it shows "No notifications."
Flags: needinfo?(jsavory)
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 8•11 years ago
|
||
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.
Flags: needinfo?(rmacdonald)
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•11 years ago
|
||
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?
Attachment #817298 -
Flags: ui-review?
Attachment #817298 -
Flags: ui-review+
Attachment #817298 -
Flags: review+
Updated•11 years ago
|
Flags: needinfo?(leese.thomas81)
Comment 11•11 years ago
|
||
Do remember to reset the review? flag to me please.
Assignee | ||
Comment 12•11 years ago
|
||
Comment on attachment 817298 [details] [review] pull request Fixed the issue with the tests, passes in Travis CI now.
Attachment #817298 -
Flags: review?(timdream)
Updated•11 years ago
|
Attachment #817298 -
Flags: review?(timdream) → review+
Comment 13•11 years ago
|
||
master: https://github.com/mozilla-b2g/gaia/commit/04c1e38c5186cb06f6e61c4b80ab1aff3d4e3ae9 Thank you again for your contribution!
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Whiteboard: [mentor=timdream]
You need to log in
before you can comment on or make changes to this bug.
Description
•