1133006 - [Calendar][Add Event] The "Add Event" header is left aligned and not centered

Status: VERIFIED FIXED

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

Description

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Description:
When creating a new event within the calendar app, the header will not be centered.

Repro Steps:
1) Update a Flame to 20150213010213
2) Open Calendar App
3) Tap "+" to Create a new event
4) Observe the "Add Event" header


Actual:
The "Add Event" header is not centered, and is found moved over to the left

Expected:
The "Add Event" header is properly centered at the top

Environmental Variables:
Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
Build ID: 20150213010213
Gaia: 2a2b008f9ae957fe19ad540d233d86b5c0b6829e
Gecko: 2f5c5ec1a24b
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Repro frequency: 10/10
See attached: Logcat, Video - http://youtu.be/k0FQ9oF4M0E

Comment 1

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

This issue DOES occur on Flame 2.2

The "Add Event" header is not centered, and is found moved over to the left

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150213002503
Gaia: ea64caf6d4ab03fc4472eca9f41f20d651d55fa9
Gecko: d04b78eeb536
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
--------------------------------------------------------------------------------------------
This issue does NOT occur on Flame 2.1

The "Add Event" header is properly centered at the top

Environmental Variables:
Device: Flame 2.1 (319mb)(Kitkat)(Full Flash)
Build ID: 20150213001208
Gaia: e8eba437af02820f74d122aec83b6001df6f89e3
Gecko: a64519246a81
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 2

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Attachment: Calendar Header Logcat

Comment 3

[Miller Medeiros [:millermedeiros]]

this is probably not calendars fault since we are using the <gaia-header> element. it looks like it's calculating the `marginStart` the wrong way - in this case it's setting `margin-left:-50px` even tho we have 2 buttons of almost the same width (left button is 50px, right button is 53px).

this was probably caused by Bug 1112131, I was going to try to fix it but since I'm going to be away the next week I'll ?ni :julienw instead; he should be able to redirect this to someone else that is more familiar with the <gaia-header> codebase.

Comment 4

[Julien Wajsberg [:julienw]]

Miller, can you please just confirm this panel is "elements/modify_event.html" ?

Comment 5

[Miller Medeiros [:millermedeiros]]

Yes it is the modify_event.html it's used by add and edit event screens. Thanks a lot.

Comment 6

[Julien Wajsberg [:julienw]]

It's properly recalculated if I force a runFontFit recalculation on the header.

So something is not correctly calculated when it's run the first time.

Comment 7

[Julien Wajsberg [:julienw]]

I think the issue is that we're calculating at startup when the panel is hidden. As a result when we get the button's width, it's equal to 0.

I'll look at this on Monday with Wilson. I don't have a good idea right now :)

Comment 8

[Peter Bylenga [:PBylenga]]

Holding off on calling for a window, if one would be helpful please request one.

Comment 9

[Miller Medeiros [:millermedeiros]]

bug is fixed after reverting commit https://github.com/mozilla-b2g/gaia/commit/cd0bfb0daf7d81467a169bc2e8513f342cf7c070 so no window needed here (that's how I got that Bug 1112131 was the cause)

Comment 10

[Julien Wajsberg [:julienw]]

I reverted the commit, and while the issue is less apparent, it's still there: the header is not correctly centered. But it's only a few pixels off in the previous version while it's 50px off with the new version.

The issue is clear: when the panel is hidden using "display: none" all measurements are wrong (widths = 0, etc). The solution is clear too: manually run the "runFontFit" method on the header when the panel is first run.
Another smaeter possibility: add the "no-font-fit" attribute (because there is no need to run the algorithm if the results are wrong) and remove it when displaying the panel for the first time (which will trigger a calculation).

Sadly it's not possible to use the title-start/end attributes because the "save" button has a dynamic width.

Gareth, Kevin, maybe one of you can have a look because Miller is in PTO ?

Comment 11

[Kevin Grandon :kgrandon]

Seems pretty trivial. I could take this.

Comment 12

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:bug_1133006_modify_event_font_fit > mozilla-b2g:master

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8564934 [details] [review]
[gaia] KevinGrandon:bug_1133006_modify_event_font_fit > mozilla-b2g:master

Either Julien or Gareth could probably review this one. Either of you guys have time? Thanks!

Comment 14

[Julien Wajsberg [:julienw]]

Comment on attachment 8564934 [details] [review]
[gaia] KevinGrandon:bug_1133006_modify_event_font_fit > mozilla-b2g:master

While this very bug is fixed, some use cases are still broken. More information on the github PR.

Also 2 nit-comments.

Comment 15

[Kevin Grandon :kgrandon]

Julien - could you provide a screenshot of what you're seeing with the add account screen? I tested and it appeared to work for me. I also wonder if we should track that in another bug.

I've addressed the nits on github.

Comment 16

[Stephany Wilkes]

Triage is taking this because this is a regression.

Comment 17

[Julien Wajsberg [:julienw]]

Attachment: issue 1 in Settings

Comment 18

[Julien Wajsberg [:julienw]]

Attachment: Issue 2 in Settings > Add Account > Yahoo

Here are the 2 screenshots.

I checked that manually calling runFontFit on both headers make them work. Maybe that for "Calendar Settings" you'll have a better way than calling the method though.

I think we should fix these 2 panels in this bug.

Comment 19

[Kevin Grandon :kgrandon]

Ok, thanks Julien. I had forgotten all of the details about the font fit, but this does make sense. I've updated the patch and will open it for review again shortly.

Comment 20

[Kevin Grandon :kgrandon]

Comment on attachment 8564934 [details] [review]
[gaia] KevinGrandon:bug_1133006_modify_event_font_fit > mozilla-b2g:master

Gareth or Miller - could one of you guys give this a review? Thanks!

Comment 21

[Miller Medeiros [:millermedeiros]]

Comment on attachment 8564934 [details] [review]
[gaia] KevinGrandon:bug_1133006_modify_event_font_fit > mozilla-b2g:master

clean enough and solves the problem. thanks a lot!

Comment 22

[Kevin Grandon :kgrandon]

Thanks for the review Miller.

Comment 23

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/2e34269bdc7ef56ccca5bea8e3470e9efcbf92ce

Comment 24

[Julien Wajsberg [:julienw]]

Kevin, we need an approval here.

Comment 25

[Kevin Grandon :kgrandon]

Comment on attachment 8564934 [details] [review]
[gaia] KevinGrandon:bug_1133006_modify_event_font_fit > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Missing since feature implementation I think.
[User impact] if declined: Inconsistent UX when it comes to headers across the OS.
[Testing completed]: Manual,and a simple unit test added.
[Risk to taking this patch] (and alternatives if risky): Low risk, small-ish patch.
[String changes made]: None.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/a4484f2a71c2ce0920ba33237475019a0ab3c8da

Comment 27

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed on the latest Nightly Flame 3.0 build.  NI? myself to check this on 2.2 after it is on nightly.

Actual Results: The Add Event header is centered. 

Environmental Variables:
Device: Flame 3.0
BuildID: 20150302010223
Gaia: f34ce82a840ad3c0aed3bfff18517b3f6a0eb37f
Gecko: eea6188b9b05
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Comment 28

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed on the latest Nightly Flame 2.2 build

Actual Results: The Add Event header is centered. 

Environmental Variables:
Device: Flame 2.2
BuildID: 20150303002527
Gaia: 3d188c414e30acc392253d5389a42352fcfbc183
Gecko: c89aad487aa5
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0