Closed Bug 1071717 Opened 10 years ago Closed 10 years ago

[Active Call Indicator] Indicator should move down with the utility tray when opened

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

x86
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1067913

People

(Reporter: epang, Assigned: apastor)

Details

(Keywords: polish, Whiteboard: [systemsfe])

Attachments

(2 files)

Hi Alberto, This bug is to update the position of the active call indicator when the notification tray is open. It should move down with the tray and overlay the handle. The animation should continue and if the tray is closed the indicator moves back up with the handle I've attached the flow, but it can also be found here on box: https://mozilla.box.com/s/b6vzr85snxplv32rpkwu Alberto has worked on something similar for the ambient indicator for notifications. Here's a link to the bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1067913 Please let me know if you have any questions. thanks! Eric
(In reply to Eric Pang [:epang] from comment #0) > Created attachment 8493875 [details] > [Spec] Ambient_Indicator_ActiveCall > > Hi Alberto, > > This bug is to update the position of the active call indicator when the > notification tray is open. > > It should move down with the tray and overlay the handle. The animation > should continue and if the tray is closed the indicator moves back up with > the handle > > I've attached the flow, but it can also be found here on box: > https://mozilla.box.com/s/b6vzr85snxplv32rpkwu > > Alberto has worked on something similar for the ambient indicator for > notifications. Here's a link to the bug: > https://bugzilla.mozilla.org/show_bug.cgi?id=1067913 > > Please let me know if you have any questions. > thanks! > > Eric Meant to say Alive, but since Alberto worked on the same change for notifcations, sorry! Alive can you help with this?
Flags: needinfo?(alive)
Hey Eric, Moving indicator with utility tray means * The height of utility tray is not fixed: it's screen height - statusbar height when there is no indicator; screen height - statusbar - indicator height when there is indicator. Is this true? * Otherwise, the position of indicator is not fixed - it's "outside" utility tray when utility tray is closed, and inside utility tray when utility tray is opening/opened. Is this true?
Flags: needinfo?(alive) → needinfo?(epang)
Attached file WIP
This patch moves indicator into utility tray and adjust the z-index/bottom dynamically. Honestly I dislike bind it with utilityTray... but let's see how you feel.
Attachment #8504483 - Flags: ui-review?(epang)
Attachment #8504483 - Flags: feedback?(etienne)
Comment on attachment 8504483 [details] [review] WIP Wow we definitely need to keep those 2 bugs in sync. Needinfo-ing Alberto to make sure he sees this. I'm afraid we're going to end up with 2 potentially complex and different implementation of what is, to the user, the same feature. This is inherently coupled with the UtilityTray, that's how the spec is built and it is the mental model we're exposing to the user. I know utility_tray.js is old and messy but maybe giving it the correct responsibilities will push us to make it better. My suggestion is the following: * have 1 ambient indicator dom node, always positioned correctly at bottom:0 of the #utility-tray, owned by UtilityTray (we'll get the "following while dragging" part for free) * have the UtilityTray expose (ideally via System.register/System.request) - showAmbientIndicator/hideAmbientIndicator - makeAmbientIndicatorActive/makeAmbientIndicatorInactive Those methods could optionally implement a simple reference counting logic. Those method will toggle CSS classes on the ambient indicator dom node (to manage visibility state + animation state) _and_ on the #utility-tray to lower it by 0.2rem when there's an ambient indicator displayed. We'll have to take this into account at the beginning of the drag gesture since we won't start from completely outside of the viewport but from 0.2rem in the view port. Obviously an "active" indicator is inherently shown but that's easy to express in CSS :) I want to stay true to the spec, the whole point of the ambient indicator is to invite the user to drag down the UtilityTray let's embrace this and lower the actual UtilityTray instead of fighting it and then having a hard time keeping the indicator and the tray in sync. What do you guys think? Alberto will you be able to take this once we have a solution we all agree on?
Attachment #8504483 - Flags: feedback?(etienne)
Flags: needinfo?(apastor)
I do agree we need a common solution for this. The main reason I didn't attach the ambient indicator to the utility tray is that, initially, it was more attached to notifications (toast) than the utility tray itself. The second argument was that in terms of accessibility, making the ambient indicator part of the utility tray made readers think that the utility tray was open when it wasn't. I'm not trying to say that I think keeping it out of the utility tray is better, but just giving a little bit of context on the decisions made. I do think that now that the ambient indicator is really attached to the utility tray, we need to think on a solution, but being aware of the accessibility issues as well. Happy to take it as soon as we agree on a solution :)
Flags: needinfo?(apastor)
Comment on attachment 8504483 [details] [review] WIP Removing UI-Reivew for now until this a consistent implementation :)
Attachment #8504483 - Flags: ui-review?(epang)
Flags: needinfo?(epang)
(In reply to Etienne Segonzac (:etienne) from comment #4) > Comment on attachment 8504483 [details] [review] > WIP > > Wow we definitely need to keep those 2 bugs in sync. Needinfo-ing Alberto to > make sure he sees this. > > I'm afraid we're going to end up with 2 potentially complex and different > implementation of what is, to the user, the same feature. > > This is inherently coupled with the UtilityTray, that's how the spec is > built and it is the mental model we're exposing to the user. I know > utility_tray.js is old and messy but maybe giving it the correct > responsibilities will push us to make it better. > > My suggestion is the following: > * have 1 ambient indicator dom node, always positioned correctly at bottom:0 > of the #utility-tray, owned by UtilityTray > (we'll get the "following while dragging" part for free) > > * have the UtilityTray expose (ideally via System.register/System.request) > - showAmbientIndicator/hideAmbientIndicator > - makeAmbientIndicatorActive/makeAmbientIndicatorInactive > > Those methods could optionally implement a simple reference counting logic. > > Those method will toggle CSS classes on the ambient indicator dom node (to > manage visibility state + animation state) _and_ on the #utility-tray to > lower it by 0.2rem when there's an ambient indicator displayed. We'll have > to take this into account at the beginning of the drag gesture since we > won't start from completely outside of the viewport but from 0.2rem in the > view port. Obviously an "active" indicator is inherently shown but that's > easy to express in CSS :) > > I want to stay true to the spec, the whole point of the ambient indicator is > to invite the user to drag down the UtilityTray let's embrace this and lower > the actual UtilityTray instead of fighting it and then having a hard time > keeping the indicator and the tray in sync. > > What do you guys think? > Alberto will you be able to take this once we have a solution we all agree > on? +1 for Etienne's idea, +100 if Alberto could steal this from me!! :DDDD
(In reply to Alberto Pastor [:albertopq] from comment #5) > The > second argument was that in terms of accessibility, making the ambient > indicator part of the utility tray made readers think that the utility tray > was open when it wasn't. > This is a really good point. We can probably get around this with an aria-hidden attribute but we should check with Eitan first :) Hi Eitan, the idea is that, when something is waiting for you in the utility tray (maybe an unread notification), the bottom 0.2rem of the tray lights up and peeks inside the view port to invite you to drag the tray. How should we set up the tray aria-wise when it's peeking?
Flags: needinfo?(eitan)
(In reply to Etienne Segonzac (:etienne) from comment #8) > (In reply to Alberto Pastor [:albertopq] from comment #5) > > The > > second argument was that in terms of accessibility, making the ambient > > indicator part of the utility tray made readers think that the utility tray > > was open when it wasn't. > > > > This is a really good point. > We can probably get around this with an aria-hidden attribute but we should > check with Eitan first :) > > Hi Eitan, the idea is that, when something is waiting for you in the utility > tray (maybe an unread notification), the bottom 0.2rem of the tray lights up > and peeks inside the view port to invite you to drag the tray. How should we > set up the tray aria-wise when it's peeking? The current way it is implemented in master could simply be kept for this case as well. The actual indicator is hidden from the user (possibly with aria-hidden, need to check). It is then assigned an accessible label, like "unread notifications", and the status bar container has aria-describedby set to the id of the indicator. This way, when a user visits the status bar, they are told that there are unread messages. Yura implemented that, and he could explain it better. I think we may need to, in the future, graduate the indicator into a visible for screen reader element. Possibly with a role of status or alert. Something to think about. But in the meantime, I think that if the current screen reader solution is maintained, we would not be regressing.
Flags: needinfo?(eitan)
Stealing! :)
Assignee: alive → apastor
(In reply to Alberto Pastor [:albertopq] from comment #10) > Stealing! :) \O/ Feel free to ping me if you have problems when integrating them.
Closing, as this is going to be implemented into 1067913
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: