Yan Or Reporter
	Description • 10 years ago

      No description provided.

	Wilson Page [:wilsonpage] Assignee
	Comment 1 • 10 years ago

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.

	(inactive after 6/18) Alive Kuo [:alive]
	Comment 2 • 10 years ago

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

	Wilson Page [:wilsonpage] Assignee
	Comment 3 • 10 years ago

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

	(inactive after 6/18) Alive Kuo [:alive]
	Comment 4 • 10 years ago

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.

	Wilson Page [:wilsonpage] Assignee
	Comment 7 • 10 years ago

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

	Przemek Abratowski [:przemek] UX
	Comment 8 • 10 years ago

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.

	Wilson Page [:wilsonpage] Assignee
	Comment 9 • 10 years ago

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

	Fernando Jiménez Moreno [:ferjm]
	Comment 10 • 10 years ago

(In reply to Wilson Page [:wilsonpage] from comment #9)
> Created attachment 8473745 [details]
> 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

	Stephany Wilkes
	Comment 11 • 10 years ago

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.

	Wilson Page [:wilsonpage] Assignee
	Comment 12 • 10 years ago

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

UPDATED

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

	(inactive after 6/18) Alive Kuo [:alive]
	Comment 13 • 10 years ago

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

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

	Wilson Page [:wilsonpage] Assignee
	Comment 14 • 10 years ago

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #13)
> Comment on attachment 8473745 [details]
> 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?

	Stephany Wilkes
	Comment 15 • 10 years ago

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!

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 16 • 10 years ago

(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.

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 17 • 10 years ago

(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.

	Wilson Page [:wilsonpage] Assignee
	Comment 18 • 10 years ago

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.

	Wilson Page [:wilsonpage] Assignee
	Comment 19 • 10 years ago

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

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

	Kevin Grandon :kgrandon
	Comment 20 • 10 years ago

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

	Wilson Page [:wilsonpage] Assignee
	Comment 21 • 10 years ago

(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.