Closed Bug 1016814 Opened 6 years ago Closed 6 years ago

[System] Update to use gaia-header

Categories

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

x86
macOS
defect
Not set

Tracking

(ux-b2g:2.1)

RESOLVED FIXED
ux-b2g 2.1

People

(Reporter: yor, Assigned: wilsonpage)

References

Details

Attachments

(3 files)

No description provided.
Blocks: gaia-header
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.
Flags: needinfo?(alive)
I am not sure what you want..grep <h1> in system/index.html doesn't help you?
Flags: needinfo?(alive)
alive: I need actual steps to follow on the device to actually view the headers from user's perspective.
Flags: needinfo?(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.
Flags: needinfo?(alive)
Attached image crash-report-header.png
Attached image crash-report.png
NI przemek to decide how attached crash report header should appear.
Flags: needinfo?(pabratowski)
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.
Flags: needinfo?(pabratowski)
Assignee: nobody → wilsonpage
Status: NEW → ASSIGNED
Attached file 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
Attachment #8473745 - Flags: feedback?(alive)
(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
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.
ux-b2g: --- → 2.1
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).
Attachment #8473745 - Flags: review?(alive)
Comment on attachment 8473745 [details] [review]
pull-request (master)

looks good, r+ if you are sure everything is fine.
Attachment #8473745 - Flags: review?(alive)
Attachment #8473745 - Flags: review+
Attachment #8473745 - Flags: feedback?(alive)
(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?
Flags: needinfo?(alive)
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!
Flags: needinfo?(alive) → needinfo?(timdream)
(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.
(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.
Flags: needinfo?(timdream) → needinfo?(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.
Flags: needinfo?(wilsonpage)
Depends on: 1055004
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Depends on: 887541
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
(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.
Depends on: 1065179
You need to log in before you can comment on or make changes to this bug.