1016814 - [System] Update to use gaia-header

Status: RESOLVED FIXED

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

Comment 1

[Wilson Page [:wilsonpage]]

alive: Can you provide steps on how to view each of the System app's header building-block instances? I can see form the markup that there are quite a few, but haven't been able to actually find them in the OS.

Comment 2

[(inactive after 6/18) Alive Kuo [:alive]]

I am not sure what you want..grep <h1> in system/index.html doesn't help you?

Comment 3

[Wilson Page [:wilsonpage]]

alive: I need actual steps to follow on the device to actually view the headers from user's perspective.

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

I don't know whole the steps..but most of them could be inspected by AppManager.
If you have difficulty for certain target you could ask around.

TrustedUI: Open marketplace, go to settings, login
Value Selector: Open UITest, go to Select, choose one
Permission Dialog: Flash the phone, open Camera
App Install Dialog: Open marketplace, install an app
App Install Cancel Dialog: Cancel the install
App Download Callback Dialog: Not sure, but you could use AppManager inspector to see it.
Emergency Callback Dialog: Not sure, but you could use AppManager inspector to see it.
Updates via Data Connection Dialog: Not sure, but you could use AppManager inspector to see it.
Setup Installed App Dialog: Not sure, but you could use AppManager inspector to see it.
IME Layout Dialog: Not sure, but you could use AppManager inspector to see it.
IME Menu Dialog: Invoke keyboard, pull down utility tray, click the keyboard notification.
SIM Card Dialog: Open settings, enable SIM PIN, reboot and unlock
App Chrome: Open Homescreen, click search, type website URL.

Comment 5

[Wilson Page [:wilsonpage]]

Attachment: crash-report-header.png

Comment 6

[Wilson Page [:wilsonpage]]

Attachment: crash-report.png

Comment 7

[Wilson Page [:wilsonpage]]

NI przemek to decide how attached crash report header should appear.

Comment 8

[Przemek Abratowski [:przemek] UX]

This header should use the settings header color, #f4f4f4. The overall page background should also be #f4f4f4 just like settings for this to look good.

Comment 9

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

Alive, if you're not the best person to feedback/review this, please pass it on. I've done my best to integrate the new gaia-header.

There are still some screens that I haven't been able to view on device, so can't clarify if things are worked correctly, including:

- icc
- entry-form
- mobile-id

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Wilson Page [:wilsonpage] from comment #9)
> Created attachment 8473745 [details] [review]
> pull-request (master)
> 
> Alive, if you're not the best person to feedback/review this, please pass it
> on. I've done my best to integrate the new gaia-header.
> 
> There are still some screens that I haven't been able to view on device, so
> can't clarify if things are worked correctly, including:
> 
> - icc
> - entry-form
> - mobile-id

You can test the mobile id screen with the UI test added on bug 1044060

Comment 11

[Stephany Wilkes]

Marking this bug for 2.1 since the web components Header is the one committed web component for 2.1. Other web components for 2.2 depend on Header going first. 

Sorry for doing this late; Hema just brought this bug to my attention today, but it reflects agreed scope and is in the agreed 2.1 plan. Just updating the flags appropriately for release tracking.

Comment 12

[Wilson Page [:wilsonpage]]

Comment on attachment 8473745 [details] [review]
pull-request (master)

UPDATED

- Found and fixed issue with gaia-header throwing exception when inside a hidden iframe (media-id).

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8473745 [details] [review]
pull-request (master)

looks good, r+ if you are sure everything is fine.

Comment 14

[Wilson Page [:wilsonpage]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #13)
> Comment on attachment 8473745 [details] [review]
> pull-request (master)
> 
> looks good, r+ if you are sure everything is fine.

I'm not familiar enough with the app to know everything is fine. Should we get someone to give it a final check?

Comment 15

[Stephany Wilkes]

Team,

This review flag is four days old and a full review is needed. We are in sprint 3 of 2.1 and this item blocks 2.1. We need reviews to move so that these items can land this week.

Alive, please reassign or review. Tim, please advise on anyone else who can assist with a full System app review so we can be thorough and manage risk here. Let me know if anything else is blocking here. Thank you, both!

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Stephany Wilkes from comment #15)
> This review flag is four days old and a full review is needed. We are in
> sprint 3 of 2.1 and this item blocks 2.1. We need reviews to move so that
> these items can land this week.

I will look at this today (keeping the needinfo) but you can't hardly say ux-b2g: 2.1 blocks 2.1 release.

Comment 17

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #16)
> I will look at this today (keeping the needinfo) but you can't hardly say
> ux-b2g: 2.1 blocks 2.1 release.

Sorry for not going back to this patch in time yesterday -- I was caught in meetings.

So I look at the code a bit and comment 13 to comment 15, and I don't think there is anything reviewers can do to move that patch forward since it's already been r+'d. If we have problem making sure everything is alright, maybe setting qa-approval to ask for a test run to verify that?

Please also note that getting this landed before FL means Wilson has to be responsible for fixing 2.1+ regression. Please only land if you have the bandwidth to do so -- we don't want crash landing on sprint 3 and suffer out-of-control regressions.

Comment 18

[Wilson Page [:wilsonpage]]

OK I think the best thing to do is land this. I will be on call to fix any regressions that may arise. Overall, I'm fairly confident.

Comment 19

[Wilson Page [:wilsonpage]]

Comment on attachment 8473745 [details] [review]
pull-request (master)

LANDED https://github.com/mozilla-b2g/gaia/commit/2081541a2177f42d99d205c4350f399e648d0c20

Comment 20

[Kevin Grandon :kgrandon]

It looks like this landed and caused us to include two copies of component_utils.js. I've sorted it out in a follow-up here: https://github.com/mozilla-b2g/gaia/commit/fe6c8aa1ceb20b7729aa0acb501a59621432da27

Comment 21

[Wilson Page [:wilsonpage]]

(In reply to Kevin Grandon :kgrandon from comment #20)
> It looks like this landed and caused us to include two copies of
> component_utils.js. I've sorted it out in a follow-up here:
> https://github.com/mozilla-b2g/gaia/commit/
> fe6c8aa1ceb20b7729aa0acb501a59621432da27

Crap, that must have slipped in one of the conflict. Sorry.