If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[CMAS] First UI implementation

RESOLVED FIXED in 2.1 S2 (15aug)

Status

Firefox OS
Gaia::Network Alerts
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: julienw, Assigned: julienw)

Tracking

unspecified
2.1 S2 (15aug)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.1, tracking-b2g:backlog)

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
steveck
: review+
Details | Review | Splinter Review
(Assignee)

Description

3 years ago
In this bug, we'll land a preliminary UI so that we can then incrementally add the necessary bits to make this work.

In this bug we won't use real CMAS alerts but will use a temporary application to schedule fake alerts.
(Assignee)

Updated

3 years ago
Blocks: 1048360
Whiteboard: [p=1]
(Assignee)

Updated

3 years ago
Blocks: 1051793
(Assignee)

Updated

3 years ago
Target Milestone: --- → 2.1 S2 (15aug)
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

3 years ago
Created attachment 8471955 [details] [review]
github PR

You need to run "make reset-gaia" to install the application on the device ("make production" does not install it).

You can launch the 2 styles using the "Network Alerts" application. In future bugs, we'll remove the application from homescreen by using role=system.

Also, the style is not final because we still didn't have the design from Fang. I only reused the style from building blocks for now.

The rationale of the architecture is that:
* we close the app as soon as we don't need anything anymore.
* we keep no state (all necessary information comes from the notification object or the alert message).

There is one thing that I'm not sure: because of bug 818000, I don't know if we'll get the system message for alert when the attention window is open. Guess we'll need to find out and possibly register the system message in all windows (alternative is obviously to _at last_ fix bug 818000...). I have an enhanced task_runner in [1] that can help with closing the window only when no task is running anymore (I don't need this in the current code so I moved it in a separate branch).

[1] https://github.com/julienw/gaia/compare/task-runner
Attachment #8471955 - Flags: review?(schung)
blocking-b2g: --- → backlog
feature-b2g: --- → 2.1
Comment on attachment 8471955 [details] [review]
github PR

Thanks for taking the first step of UI implementation! This gives UX a clear idea about how it works with current system behavior. Mostly are good except some questions in notification onerror and request permission, which I left my concern on github.
Another question is about the attention screen shows up but notification doesn't create yet. What if user force to restart the phone or app killed by OOP at this moment? It seems that we couldn't save this alert message properly. Is that expected behavior? or could we fire the notification when the attention screen shows up?
I've seen a weird behavior is when we remove the notifiction manually and restart the phone, the notification is still there, but when we restart phone again, notification disappeared then. Is that a bug in notification? But I've tried on other apps and they didn't have this issue.
Attachment #8471955 - Flags: review?(schung)
(Assignee)

Comment 3

3 years ago
(In reply to Steve Chung [:steveck] from comment #2)
> Comment on attachment 8471955 [details] [review]
> github PR
> 
> Thanks for taking the first step of UI implementation! This gives UX a clear
> idea about how it works with current system behavior. Mostly are good except
> some questions in notification onerror and request permission, which I left
> my concern on github.

I removed the "requestPermission" call (I think I've originally put it to try in Firefox) and since we send the notification earlier now I don't think it makes sense to do anything in the onerror (except logging something).

> Another question is about the attention screen shows up but notification
> doesn't create yet. What if user force to restart the phone or app killed by
> OOP at this moment? It seems that we couldn't save this alert message
> properly. Is that expected behavior? or could we fire the notification when
> the attention screen shows up?

Yeah, I did just this in the new version. Firing the notification later was a left-over from a previous version of the patch.

> I've seen a weird behavior is when we remove the notifiction manually and
> restart the phone, the notification is still there, but when we restart
> phone again, notification disappeared then. Is that a bug in notification?
> But I've tried on other apps and they didn't have this issue.

Yeah I've seen it too. There was a related bug in the notification subsystem (bug 1052435), I think it was fixed yesterday. It was not _exactly_ the same issue, so let's keep a look though. For example I've seen the issue in the latest nightly for Buri, but I'm not sure the patch was in. Maybe there is another issue, but it's definitely in the Notification subsystem if that happens.
(Assignee)

Comment 4

3 years ago
Comment on attachment 8471955 [details] [review]
github PR

I added a separate commit to make your review easier. You'll also find the new icons from Fang.

Please squash and merge yourself if it's r+ because I don't work this friday :)
Attachment #8471955 - Flags: review?(schung)
Comment on attachment 8471955 [details] [review]
github PR

r=me and I'll merge the patch later, thanks!
Attachment #8471955 - Flags: review?(schung) → review+
in master : 4d8f302e28d97ca36ed61407a2f07e6a7fd23716
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Component: Gaia::SMS → Gaia::Network Alerts
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.