Closed Bug 1207093 Opened 4 years ago Closed 3 years ago

[Messages][Dialer] Implement low storage condition popup

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: julienw, Unassigned)

References

Details

(Whiteboard: [p=2])

Attachments

(1 file)

See spec in blocking bug.
Assignee: nobody → felash
Comment on attachment 8664319 [details] [review]
[gaia] julienw:1207093-low-storage-popup > mozilla-b2g:master

So this is a WIP. The basic structure is here but it doesn't work properly yet. I hope we can make it work in a web component structure.

Because I'm not here tomorrow, may I have your early feedback ? Do you think this is the right direction ?
Attachment #8664319 - Flags: feedback?(gsvelto)
Attachment #8664319 - Flags: feedback?(azasypkin)
Comment on attachment 8664319 [details] [review]
[gaia] julienw:1207093-low-storage-popup > mozilla-b2g:master

I've tried to run the example application in nightly but couldn't get it to work either. I've glanced over the code and I really like the idea of having this as a web component but having never written one I'm not really sure what needs to be done to make it work properly.
Attachment #8664319 - Flags: feedback?(gsvelto)
Comment on attachment 8664319 [details] [review]
[gaia] julienw:1207093-low-storage-popup > mozilla-b2g:master

I like the idea as well. I've left one question regarding "gaiaComponent.register" method and couldn't run example in browser as well.

I see that if we use gaia-buttons/gaia-confirm as part of ShadowDOM their createdCallback's are not called that leads to a broken component we see in Firefox now, and it seems OK if they're part of Light DOM. But I have no clue if it's known issue or we just should add some "magic" to make it work :)

Thanks!
Attachment #8664319 - Flags: feedback?(azasypkin)
Depends on: 1208113
It works in Firefox now but I get bug 1208113 (that I could workaround). Also maybe I won't encounter the issue if I use the newer way of doing a webcomponent. I'll try the new way tomorrow.
Comment on attachment 8664319 [details] [review]
[gaia] julienw:1207093-low-storage-popup > mozilla-b2g:master

OK, I think it's fairly ready for a review.

You can look at it on device by going to this URL, that has latest version:
http://everlong.org/mozilla/test-component/shared/elements/gaia_lowstorage_dialog/examples/

The transition in the example in Firefox looks bad because of bug 1208456 but it looks OK on device.

I decided to use an "old style" web component (using component_utils) because it's based on gaia_confirm which is "old style" -- the "new style" dialog is using the new style BB.

I kept the same interface as the underlying gaia_confirm (using the "hidden" attribute) but tell me if you'd prefer that I use the interface of the new style gaia-dialog instead (with methods show/hide) to make it easier to migrate later on.

BTW I also started a "new style webcomponent" in https://github.com/julienw/gaia-lowstorage-dialog, but the example is not working yet.
Attachment #8664319 - Flags: review?(gsvelto)
Attachment #8664319 - Flags: review?(azasypkin)
Attachment #8664319 - Flags: feedback?(kevingrandon)
Comment on attachment 8664319 [details] [review]
[gaia] julienw:1207093-low-storage-popup > mozilla-b2g:master

Looks good to me. Thanks!
Attachment #8664319 - Flags: feedback?(kevingrandon) → feedback+
From Katie, we should change the texts to:

Title: Low Device Storage
1: Less than 10MB remain.
2: Messages can not be sent, saved or received, nor can contacts be added at this time.
3: At least 10 MB of space is required. Delete apps, then restart your device.


I pushed a new version with these changes:
* follow this spec change
* use <br> instead of <div> because <div>s can't be inside <p> and while it works currently I think it may break in the future if imported content follows HTML5 rules better (for example if I add a <div> juste before <content>, it ends the <p> because <p> can't contain <div> and as a result it's not imported by gaia_confirm)
* move the subtitle "less than 5MB" into the component rather from the component user. In the future the value "5MB" may be changed automatically by the component depending on the low storage status we're in.
* follow Kevin advice: I modified the "strong" selector in gaia_confirm instead of introducing a new ".stronger" selector.
Comment on attachment 8664319 [details] [review]
[gaia] julienw:1207093-low-storage-popup > mozilla-b2g:master

Generally looks good to me and I'm really looking forward to seeing it in action. I've just left a comment about the event handler, nothing major though.
Attachment #8664319 - Flags: review?(gsvelto) → review+
Hey Katie, one more question for you.

When the user taps on "learn more" to enter Settings, then presses the top left "back" button (which ends the activity and moves back to the application), what should he see ? Still the dialog, or should the dialog be closed ?
Flags: needinfo?(kcaldwell)
Comment on attachment 8664319 [details] [review]
[gaia] julienw:1207093-low-storage-popup > mozilla-b2g:master

Looks good to me, just few nits and questions at GitHub (that you've mostly answered already)!

Thanks!
Attachment #8664319 - Flags: review?(azasypkin) → review+
(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #10)
> Hey Katie, one more question for you.
> 
> When the user taps on "learn more" to enter Settings, then presses the top
> left "back" button (which ends the activity and moves back to the
> application), what should he see ? Still the dialog, or should the dialog be
> closed ?

Hey Julien,

Good question. Tapping "Learn More" should open Settings as an overlay ‘on top’ of Messages app. Tapping the ‘x’ closes (not a back arrow) the Settings overlay and returns the user to the previous screen with Low Storage Notice open (as user left it). 

[UX spec updates will get posted to box today.]
Flags: needinfo?(kcaldwell)
(In reply to katieC from comment #12)
> (In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #10)
> > Hey Katie, one more question for you.
> > 
> > When the user taps on "learn more" to enter Settings, then presses the top
> > left "back" button (which ends the activity and moves back to the
> > application), what should he see ? Still the dialog, or should the dialog be
> > closed ?
> 
> Hey Julien,
> 
> Good question. Tapping "Learn More" should open Settings as an overlay ‘on
> top’ of Messages app. Tapping the ‘x’ closes (not a back arrow) 

Currently the Settings activity opens Settings with a "back" button. (it always seemed weird to me BTW, but it's like this and I doubt it will change for 2.5 now, sorry :( ).

> the Settings
> overlay and returns the user to the previous screen with Low Storage Notice
> open (as user left it). 

Thanks !
Katie, I have another question for you.

Currently, in english, the text fits nicely the page.
However, if the text takes more space in other locale, what should we do?
I guess we should scroll, but what part should scroll?
My take is that the title, buttons, and "learn more" link should stay always visible, and as a result scroll the bold and normal text only.

What do you think ?
Flags: needinfo?(kcaldwell)
> Currently the Settings activity opens Settings with a "back" button. (it
> always seemed weird to me BTW, but it's like this and I doubt it will change
> for 2.5 now, sorry :( ).

That's odd, as I asked the UX team what the pattern is for that and heard an Overlay has either and "x" or a "Done" button. I'll find out more...

NI'ing Amy about the scrolling pattern for dialogue windows (but your take sounds correct to me). Amy see Julien's comment 14. thanks!
Flags: needinfo?(kcaldwell) → needinfo?(amlee)
(In reply to katieC from comment #15)
> > Currently the Settings activity opens Settings with a "back" button. (it
> > always seemed weird to me BTW, but it's like this and I doubt it will change
> > for 2.5 now, sorry :( ).
> 
> That's odd, as I asked the UX team what the pattern is for that and heard an
> Overlay has either and "x" or a "Done" button. I'll find out more...
> 
> NI'ing Amy about the scrolling pattern for dialogue windows (but your take
> sounds correct to me). Amy see Julien's comment 14. thanks!

Hi Katie, 

I haven't encountered a dialogue that required scrolling. Might be a good idea to confirm with the UX team to see if there is a rule for this that I'm not aware of.
Flags: needinfo?(amlee)
Hey Julien,

Just chatted about the scroll pattern with fellow UXers. As you said, Title and Buttons are static. Body copy (including the Learn More link) are scrollable.
Hey Julien - in reply to Comment 13

If we can't change the '<' back to an 'x' close button, then, let's change the string:
from Learn More to Go to Settings

and...

then instead of an overlay for the Settings Activity within the Messages App, it becomes an Open App activity, where the Message app minimizes in Task Manager and Settings App opens (with small icon), slides in from the side and maximizes to full screen, showing Application Storage.


..........................
UX Rationale:
We redirect the user to Settings area to understand their storage usage (ideally, learning that in the future they can come here to monitor their usage). By opening the Settings App, we allow the user to navigate to to other areas of Settings, including App Permissions (or even Media Storage). The user *may* know Permissions is an area they can uninstall apps or clear the cache/history to free up internal storage space.
Hey Katie,

We can't do that currently, sorry :(
We can only use the "configure" activity that exists today, and that opens the Settings app in some way with some behavior. This activity works like I said earlier.

Note it's the exact same activity used when we press the "Settings" button in the Utility Tray, or that happens when we press the "Wifi" buttons and it gets enabled, or when we're in the SMS application and triggers the "Settings" button, or when we're offline and get an error dialog and click "check network parameters" (or smthg).

I _definitely_ think it should be a 'x' button, not a 'back' button. I'll file a separate bug for this.

About comment 17, the issue I see with scrolling also the "Learn More" link is that it may not be obvious there actually _is_ a "Learn More" link if it's hidden but the whole text is visible.

That said, it's more difficult to do it like this than to do what you suggest, so I can do like you propose now, and try later to make it like I propose (if you and other UX folks agree of course).
Flags: needinfo?(kcaldwell)
I filed bug 1209870 about it.
(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #19)
> Hey Katie,
> 
> We can't do that currently, sorry :(
> We can only use the "configure" activity that exists today, and that opens
> the Settings app in some way with some behavior. This activity works like I
> said earlier.

I saw there is a "window" configure activity as well, that we could use. So we could do what you say after all, if that activity works as we expect.

But I'd rather see the "x" button.

> Note it's the exact same activity used when we press the "Settings" button
> in the Utility Tray,

Sorry, not this one ;)
Hey Julien,

Thanks for always being so quick to respond and explain what's possible within our constraints. :) If we can do the "x" - that is the ideal. Comment 18, is only meant to be an alternative, if we can't change the '<' to a 'x' close.

As for Comment 17/19 re: Learn More link. I get what you're saying, but the (as you said) complexity of having the link static, introduces a new pattern/behaviour. At this time, I recommend keeping it consistent with other dialogue windows. Also, the 2nd paragraph of copy is providing the user with more information about the problem (10MB space req.) and what action to take (delete apps, restart device) to fix the problem (echoing the message in 'Learn More' link to Application Storage).
Flags: needinfo?(kcaldwell)
Hi all,

Matej did a string review and we have an update. 
IxD and VsD Specs will be updated to reflect this change.

Please update strings from 'Low Device Storage' to 'Critically Low Storage'

Impacts:
• Settings App: Application Storage, Media Storage & App Permission
• System App: Lockscreen, System Toast & Utility Tray Notifications
• Message App: Low Storage Notice
• Dialer App: Low Storage Notice and Incoming/Outgoing Call Notifications
It won't be implemented soon. The patch here should apply cleanly but it's not really useful to land it now. Better wait and reimplement this on top of the newer fxos-dialog when we need it.
Assignee: felash → nobody
Mass closing of Gaia::SMS bugs. End of an era :(
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Mass closing of Gaia::SMS bugs. End of an era :(
You need to log in before you can comment on or make changes to this bug.