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

VERIFIED FIXED in Firefox OS v2.2

Status

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: dharris, Assigned: kgrandon)

Tracking

({regression})

unspecified
2.2 S7 (6mar)
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [3.0-Daily-Testing][systemsfe], URL)

Attachments

(4 attachments)

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
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Created attachment 8564298 [details]
Calendar Header Logcat
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.
Blocks: 1112131
Flags: needinfo?(felash)
Miller, can you please just confirm this panel is "elements/modify_event.html" ?
Flags: needinfo?(mmedeiros)
Yes it is the modify_event.html it's used by add and edit event screens. Thanks a lot.
Flags: needinfo?(mmedeiros)
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.
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 :)
blocking-b2g: --- → 2.2?
Flags: needinfo?(felash)
Flags: needinfo?(felash)
Holding off on calling for a window, if one would be helpful please request one.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
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)
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 ?
Flags: needinfo?(kgrandon)
Flags: needinfo?(gaye)
Flags: needinfo?(felash)
(Assignee)

Comment 11

4 years ago
Seems pretty trivial. I could take this.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Flags: needinfo?(kgrandon)
Flags: needinfo?(gaye)
Created attachment 8564934 [details] [review]
[gaia] KevinGrandon:bug_1133006_modify_event_font_fit > mozilla-b2g:master
(Assignee)

Comment 13

4 years ago
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!
Attachment #8564934 - Flags: review?(gaye)
Attachment #8564934 - Flags: review?(felash)
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.
Attachment #8564934 - Flags: review?(gaye)
Attachment #8564934 - Flags: review?(felash)
(Assignee)

Comment 15

4 years ago
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.
Flags: needinfo?(felash)

Comment 16

4 years ago
Triage is taking this because this is a regression.
blocking-b2g: 2.2? → 2.2+
Created attachment 8567939 [details]
issue 1 in Settings
Created attachment 8567945 [details]
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.
Flags: needinfo?(felash)
(Assignee)

Comment 19

4 years ago
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.
(Assignee)

Comment 20

4 years ago
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!
Attachment #8564934 - Flags: review?(mmedeiros)
Attachment #8564934 - Flags: review?(gaye)
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!
Attachment #8564934 - Flags: review?(mmedeiros)
Attachment #8564934 - Flags: review?(gaye)
Attachment #8564934 - Flags: review+
Flags: needinfo?(kgrandon)
(Assignee)

Comment 22

4 years ago
Thanks for the review Miller.
Flags: needinfo?(kgrandon)
Keywords: checkin-needed

Updated

4 years ago
Keywords: checkin-needed

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Kevin, we need an approval here.
Flags: needinfo?(kgrandon)
(Assignee)

Comment 25

4 years ago
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.
Flags: needinfo?(kgrandon)
Attachment #8564934 - Flags: approval-gaia-v2.2?(bbajaj)

Updated

4 years ago
Attachment #8564934 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/a4484f2a71c2ce0920ba33237475019a0ab3c8da
status-b2g-v2.2: affected → fixed
status-b2g-master: affected → fixed
Target Milestone: --- → 2.2 S7 (6mar)
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
status-b2g-master: fixed → verified
Flags: needinfo?(ktucker)
Flags: needinfo?(jmercado)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Keywords: verifyme
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
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
status-b2g-v2.2: fixed → verified
Flags: needinfo?(jmercado) → needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Keywords: verifyme
Whiteboard: [3.0-Daily-Testing] → [3.0-Daily-Testing][systemsfe]
You need to log in before you can comment on or make changes to this bug.