Closed
Bug 1067913
Opened 10 years ago
Closed 10 years ago
[Ambient Indicator] Ambient indicator should move down with the utility tray when opened
Categories
(Firefox OS Graveyard :: Gaia::System, defect, P1)
Tracking
(tracking-b2g:backlog)
RESOLVED
FIXED
tracking-b2g | backlog |
People
(Reporter: epang, Assigned: apastor)
References
()
Details
(Keywords: polish, Whiteboard: [systemsfe])
Attachments
(2 files, 2 obsolete files)
Hi Alberto,
This bug is to update the position of the ambient indicator when the notification tray is open.
It should move down with the tray and overlay the handle - once at the bottom it fades out (same as the current behavior at the top).
I've attached the flow, but it can also be found here on box: https://mozilla.box.com/s/jaylzmx4s2eg1yp08m4m
Please let me know if you have any questions.
thx
Eric
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
Eric, could you please try this patch?
https://github.com/mozilla-b2g/gaia/pull/24230
I'm not sure what should be the behaviour regarding the statusbar color when starting the process of opening the utility tray. Please, let me know what you think.
Thanks
Flags: needinfo?(epang)
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to Alberto Pastor [:albertopq] from comment #1)
> Eric, could you please try this patch?
>
> https://github.com/mozilla-b2g/gaia/pull/24230
>
> I'm not sure what should be the behaviour regarding the statusbar color when
> starting the process of opening the utility tray. Please, let me know what
> you think.
>
> Thanks
This is good, exactly what I was expecting :). If you can flag me on ui-review I can r+
Thanks!
Flags: needinfo?(epang)
Assignee | ||
Updated•10 years ago
|
Attachment #8489956 -
Flags: ui-review?(epang)
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 8489956 [details]
[Flow] Notifications - Ambient Indicator
Looks good, thanks for updating this Alberto!
Attachment #8489956 -
Flags: ui-review?(epang) → ui-review+
Assignee | ||
Comment 4•10 years ago
|
||
Will add some tests before r? Thanks!
Assignee | ||
Comment 5•10 years ago
|
||
Sorry, I updated the worng attachment. Could you please ui-r+ this one? Thanks!
Attachment #8493156 -
Flags: ui-review?(epang)
Assignee | ||
Updated•10 years ago
|
Attachment #8493156 -
Flags: review?(mhenretty)
Reporter | ||
Updated•10 years ago
|
Attachment #8489956 -
Flags: ui-review+
Reporter | ||
Updated•10 years ago
|
Attachment #8493156 -
Flags: ui-review?(epang) → ui-review+
Comment 6•10 years ago
|
||
Comment on attachment 8493156 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24230
Code looks good to me. One small nit I have is that it would have made sense to attach the ambient indicator to the bottom of the utility tray in the DOM so that you don't have to translate both elements independently. But perhaps that is trickier than I imagine, so I won't block the review on it.
I do have issues with the UX of this change though. I think it decreases the overall polish of the utility tray. The reason is the statusbar. Before this change, the utility tray (including grippy) would appear behind the statusbar. This somewhat hid the abrupt change in color and text of the statusbar when opening the tray because the tray would slide in behind statusbar and therefore feel very integrated with it. But now when you start to pull down on the utility tray, the statusbar changes color, but the utility tray appears on top of it. This looks really strange when you only have the uility tray open a little or when peeking at notifications and then closing the tray (ie, you will see the utility tray slide up over the statusbar, and then a moment later the statusbar color and text will change). It looks worse than before imho.
One solution would be to have a moz-element statusbar that gets revealed with the rest of the utility tray rather than transitioning the way we currently do.
Just to clarify, this patch doesn't cause the issue I describe above. It's always been around. It just makes the issue way more apparent.
Eric, can you take another look at this change. Specifically, check out the transition when you have the ambient indicator and you peek at notifications but then close it. Are we sure we want to land it like this? Since this is a polish thing, my thinking is we should work on the statusbar transition as well.
Attachment #8493156 -
Flags: review?(mhenretty)
Flags: needinfo?(epang)
Assignee | ||
Comment 7•10 years ago
|
||
I agree that the animation is not optimal. As per my first comment (https://bugzilla.mozilla.org/show_bug.cgi?id=1067913#c1) I wasn't totally happy with the interaction between the ambient indicator and the statusbar. Moving the handle above the statusbar helps to understand better the ambient indicator transition, but degrades the statusbar animation (in both cases, just when start pulling down the handle, and when releasing it, the transition is quite poor).
@mhenretty, regarding moving the ambient indicator inside the tray, it has some accesibility problems (utility tray visible when is not). At the other hand, I thought that the ambient indicator shouldn't be DOM attached to the utility tray, as they are not directly part of the same concept, but this is arguable.
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Alberto Pastor [:albertopq] from comment #7)
> I agree that the animation is not optimal. As per my first comment
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1067913#c1) I wasn't totally
> happy with the interaction between the ambient indicator and the statusbar.
> Moving the handle above the statusbar helps to understand better the ambient
> indicator transition, but degrades the statusbar animation (in both cases,
> just when start pulling down the handle, and when releasing it, the
> transition is quite poor).
>
> @mhenretty, regarding moving the ambient indicator inside the tray, it has
> some accesibility problems (utility tray visible when is not). At the other
> hand, I thought that the ambient indicator shouldn't be DOM attached to the
> utility tray, as they are not directly part of the same concept, but this is
> arguable.
Ideally we would have the Utility bar handle (grippy) as the top element of the screen. As it is dragged down anything above the handle is revealed, if the handle is released the opposite happens. Alberto, from your comment I'm not sure if this is possible, would it be? What are the main issues?
Flags: needinfo?(epang)
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8493156 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24230
Eric, just updated the patch according to your comment. Let me know if that was what you were expecting. Thanks!
Attachment #8493156 -
Flags: ui-review+ → ui-review?(epang)
Reporter | ||
Comment 10•10 years ago
|
||
Comment on attachment 8493156 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24230
Thanks Alberto! Looks good, just the polish we talked about, but I'll open a separate bug for it.
Attachment #8493156 -
Flags: ui-review?(epang) → ui-review+
Assignee | ||
Comment 11•10 years ago
|
||
Hey Eric, could you please review again. I needed to rebase from current master, and I want to make sure everything looks as expected.
Attachment #8493156 -
Attachment is obsolete: true
Attachment #8504125 -
Flags: ui-review?(epang)
Updated•10 years ago
|
Reporter | ||
Comment 12•10 years ago
|
||
Comment on attachment 8504125 [details]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25094
Removing UI-Reivew for now until there is a consistent implementation decided on :)
Attachment #8504125 -
Flags: ui-review?(epang)
Assignee | ||
Comment 13•10 years ago
|
||
I want to hear Alive's feedback here, as this patch applies to Bug 1071717 as well.
Attachment #8504125 -
Attachment is obsolete: true
Attachment #8509564 -
Flags: feedback?(alive)
Comment 14•10 years ago
|
||
Comment on attachment 8509564 [details]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25414/files
f+ for the attentionIndicator change. Not sure if etienne has any thoughts?
Attachment #8509564 -
Flags: feedback?(etienne)
Attachment #8509564 -
Flags: feedback?(alive)
Attachment #8509564 -
Flags: feedback+
Comment 15•10 years ago
|
||
Comment on attachment 8509564 [details]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25414/files
It's shaping up nicely :)
We still plan on having the attention indicator use a System.request instead of directly manipulating the DOM from the UtilityTray right?
Attachment #8509564 -
Flags: feedback?(etienne) → feedback+
Updated•10 years ago
|
blocking-b2g: --- → backlog
Priority: -- → P1
Assignee | ||
Updated•10 years ago
|
Attachment #8509564 -
Flags: review?(etienne)
Comment 17•10 years ago
|
||
Comment on attachment 8509564 [details]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25414/files
Looking good!
Forwarding the final review to Guillaume since he knows the UtilityTray well and this will serve as a nice introduction to |System.register/request|
Cheers!
Attachment #8509564 -
Flags: review?(gmarty)
Attachment #8509564 -
Flags: review?(etienne)
Attachment #8509564 -
Flags: feedback+
Comment 18•10 years ago
|
||
Comment on attachment 8509564 [details]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25414/files
Wow! Good job. Neat UI and cleaner code.
Attachment #8509564 -
Flags: review?(gmarty) → review+
Assignee | ||
Comment 19•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 20•10 years ago
|
||
this made the statusbar icons inaccessible when in utility tray
Comment 21•10 years ago
|
||
Comment 22•10 years ago
|
||
wrong bug , bug 1135233
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•