979487 - [calendar] - Update header design to accommodate larger back-button

Status: RESOLVED INVALID

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

Comment 1

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

Attachment: patch for Gaia/master

Have in mind that this patch depends on bug 979473 and bug 979478

Comment 2

[Stephany Wilkes]

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

Hi Kevin: Please review for the impact on Calendar or reassign as appropriate. Thanks!

Comment 3

[Kevin Grandon :kgrandon]

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

Pavel - I need you to land the dependent bugs first, or provide a branch that has all of the necessary changes in a single place.

Comment 4

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

Hey Kevin,

we need to land all of the bugs together because otherwise we will break all apps.
Here is the branch https://github.com/pivanov/gaia/tree/bb-headers who contains patches from bugs:
  - 979473 [BB]Headers
  - 979478 [BB]Edit_mode
  - 979487 [calendar]

hope this helps

P.S. if you need a branch with all patches for Header changes just ping me :) Thanks in advance :)

Comment 5

[Kevin Grandon :kgrandon]

Sure - but the patch as-is is not easy to review. I think we just haven't really put processes into place for patches like these, but I guess my recommendation would be to have a pull request with multiple commits from the other bugs. (Then all changes are in a single pull request, and as things land you can rebase the branch against master).

Anyway - I'll get to this today, but I do think we need better processes for this type of work.

Comment 6

[Kevin Grandon :kgrandon]

Ok - I took a look, and unfortunately I can't R+ this yet. The headings for the settings tray looks mis-aligned, and also the "Add an Account" header also looks mis-aligned. I'm not a ux designer so I can't ui-review, but I would highly doubt that this is acceptable from a ui-review. I'll try to upload screenshots but my phone is having usb/network problems, ugh. Please let me know if you can't reproduce.

Comment 7

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

Hey Kevin I agree with you about the process ... it's very hard to keep up to date all PR but I think that we can do something smarter next time.

as for the misaligned headings I think Casey can help us but for v1.4 this is the spec for the [BB]Headers ... we will do some JS tricks for v1.5

Comment 8

[Kevin Grandon :kgrandon]

Can you show me the spec? The headers just look wrong for the tray and oauth flows. The others look fine - so I highly doubt that those headers are correct.

I also disagree that we should have any JS for headers - it's just not needed and we can augment the platform if we are missing layout power in CSS.

Comment 9

[Kevin Grandon :kgrandon]

Attachment: Calendar Tray /w Patch.png

Patryk - Can you please verify the appearance of the header in the calendar tray? Thanks!

Comment 10

[Kevin Grandon :kgrandon]

Attachment: Add Account /w patch.png

Patryk - Also can you verify the appearance of the header on this page? Thanks!

Comment 11

[Casey Yee]

Kevin, is this a Js solution?   The screen shot in Comment 10 should align the header text to the center of the screen.  It looks to be centered to available space.

Comment 12

[Patryk Adamczyk [:patryk] UX]

Comment on attachment 8387051 [details]
Add Account /w patch.png

Kevin, yes this looks as expected without the JS fix.

Comment 13

[Patryk Adamczyk [:patryk] UX]

Comment on attachment 8387050 [details]
Calendar Tray /w Patch.png

Looks correct.

Comment 14

[Kevin Grandon :kgrandon]

I am confused. Why are we talking about a JS fix? We don't need a JS fix to do any centering here, and I think this looks bad as it's not centered, and the tray does not follow scan lines.

Patryk/Pavel - Is the opinion that we need a JS fix for centering because this is not the case at all. I can do a PoC to prove this fact.

Comment 15

[Kevin Grandon :kgrandon]

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

I don't think this looks very good, but I suppose I am ok with landing if visual guys are happy. We should absolutely not use javascript for this layout though - there is no need for this, so ping me if I can help or if you have questions.

Comment 16

[Patryk Adamczyk [:patryk] UX]

It should be centered. I was lead to believe we need a JS fix for that. If we don't sweet!

Comment 17

[Patryk Adamczyk [:patryk] UX]

Attachment: Ideal Solution (look at the JS fix column)

Kevin can you make it look like the JS Fix column without the JS changes? And if the label is took long (when it ellipsis) can it then be aligned off center to maximize the space available?

Comment 18

[Kevin Grandon :kgrandon]

(In reply to Patryk Adamczyk [:patryk] UX from comment #17)
> Created attachment 8387913 [details]
> Ideal Solution (look at the JS fix column)
> 
> Kevin can you make it look like the JS Fix column without the JS changes?
> And if the label is took long (when it ellipsis) can it then be aligned off
> center to maximize the space available?

Yes - we can do this. We may need to change dom structure, or be clever about previous/next element selectors if we want a cleaner dom. Which bug is this work being done in? I can take a look at the patch and provide some more details. Is it bug 979473?

Comment 19

[Patryk Adamczyk [:patryk] UX]

Bug 979473 is the meta for the feature. Casey and Pavel did all the work for it so far, so please sync with them on any new code you're trying out.

Comment 20

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

we don't need to change the markup anymore