947099 - [Dialer] Update tab bar to new 1.3 visual designs

Status: RESOLVED FIXED

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

Description

[Eric Pang [:epang] UX]

Attachment: Dialer tab bar specs.png

There are a couple of updates needed for dialer

1. Update tab bars to new 1.3 visual designs.
2. Update layout to allow for a 45px tab bar (instead of 40px)

See attachments.

Comment 1

[Eric Pang [:epang] UX]

Attachment: Daily Layout Update.png

Layout update specs

Comment 2

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

Comment 4

[Eric Pang [:epang] UX]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

Hi Fabrice, can you help review Pavel's patch? thanks in advance :)

Comment 5

[Eric Pang [:epang] UX]

Hey Antony, I just wanted to note that the patch from this bug needs is dependent with the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=947093

They will land together along with all other toolbar updates to remain consistent. Thanks!

Comment 6

[Anthony Ricaud (:rik)]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

When I apply this patch on top of the patch from bug 947093, I can't see the blue bar at the bottom of the selected tab.

Also, why are we losing 5 precious pixels of vertical space for that bar?

Comment 7

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

Hey Anthony,

sorry for the mistake was I typo (merge the wrong branch) anyway ... I think now it's ready for review. Thanks :)

Comment 8

[Anthony Ricaud (:rik)]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

One of the two changes are breaking one thing:
1) Go to the contacts tab
2) Click the "add a contact" button

Expected:
The tab bar should disappear.

Actual:
The content of the tab disappears but we still have a black 45px bar in the bottom.

This is also changing the background of the Call log header background but I don't know if that's intended.

I'm on PTO for the next three weeks so you should ask a review to someone else.

Comment 9

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

Hey Fernando,
can you take a look if you have time :). If not I will ask Etienne because we try to land this soon

Thanks :)

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

Hi Pavel! I am afraid that I am still seeing the visual issues that Anthony mentioned in comment 6.

Comment 11

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Screenshot

This is what I am seeing after applying your changes from https://github.com/pivanov/gaia/commit/343ad70664c5d88dd309feceb92e343592ccd4e0

Comment 12

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Fernando,
this but depends on Bug 947093 did you apply both?

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Screenshot

Oh, there you go! Now it looks better! :)

Unfortunately, the bottom bar does not entirely disappear (only the icons do) and I am also seeing this issue when trying to add a new contact. Check the current call log version, the bottom bar needs to disappear entirely when adding a contact.

I also added a comment to https://github.com/pivanov/gaia/commit/3b3c795925ce11e5ad25aa112775934536343e78

Thanks Pavel!

Comment 14

[Fernando Jiménez Moreno [:ferjm]]

And sorry for the delay replying to comment 12. I usually do a complete bugmail pass once a day (first thing in the morning) and I only look at review/feedback/needinfo requests for the rest of the day. So if you want me to get back to this before tomorrow, set any of this flags on me, please :).

Comment 15

[Eric Pang [:epang] UX]

(In reply to Fernando Jiménez Moreno [:ferjm] (use needinfo instead of CC, please) from comment #14)
> And sorry for the delay replying to comment 12. I usually do a complete
> bugmail pass once a day (first thing in the morning) and I only look at
> review/feedback/needinfo requests for the rest of the day. So if you want me
> to get back to this before tomorrow, set any of this flags on me, please :).

Thanks for reviewing Fernando.  The bottom toolbar should stay when adding a new contacts but like Fernando noticed the icons should be show.  Thanks!

Comment 16

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

ahhh ... I merge the wrong branch ... sorry guys ... now everything should work (I test it few times)

Comment 17

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

Thanks Pavel!

Comment 18

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Landed to master:
https://github.com/mozilla-b2g/gaia/commit/ac1f7b71eaf1858c5a3f7ef87b55257dc4f9310e

Comment 19

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Doesn't this need to land in v1.3 branch in order for it to be marked fixed?  I'm not sure what the protocol is; I am reopening this bug unless, there's a different way to track the bug being fixed for v1.3 branch.

Comment 20

[Stephany Wilkes]

This should have landed in 1.3.

Comment 21

[Dylan Oliver [:doliver]]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #19)
> Doesn't this need to land in v1.3 branch in order for it to be marked fixed?
> I'm not sure what the protocol is; I am reopening this bug unless, there's a
> different way to track the bug being fixed for v1.3 branch.

No, RESOLVED/FIXED indicates that it has landed on master. The scripts that will eventually uplift this to the proper branch depend on that status. 

The right way to get this into the 1.3 conversation is to nominate it as kaze just did. If it gets approved for inclusion in 1.3 (via blocking-b2g:1.3+), the uplift scripts will land it to the 1.3 branch and this bug will get marked with "status-b2g-v1.3:fixed". 

Can you restore the FIXED status to this bug and the related issues under bug 947087?

Comment 22

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 23

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Needs approval.  Please approve?

Comment 24

[Preeti Raghunath(:Preeti)]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

approved for 1.3 gaia per triage

Comment 25

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Comment on attachment 8343603 [details] [review]
patch for Gaia/master

We are no longer taking the tab change in 1.3; we will have it in 1.4