Closed Bug 1133006 Opened 6 years ago Closed 6 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

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

VERIFIED FIXED
2.2 S7 (6mar)
blocking-b2g 2.2+
Tracking Status
b2g-v2.1 --- unaffected
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: dharris, Assigned: kgrandon)

References

()

Details

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

Attachments

(4 files)

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)
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)
Seems pretty trivial. I could take this.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Flags: needinfo?(kgrandon)
Flags: needinfo?(gaye)
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)
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)
Triage is taking this because this is a regression.
blocking-b2g: 2.2? → 2.2+
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)
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 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)
Thanks for the review Miller.
Flags: needinfo?(kgrandon)
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Kevin, we need an approval here.
Flags: needinfo?(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.
Flags: needinfo?(kgrandon)
Attachment #8564934 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8564934 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
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?]
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?]
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.