Closed Bug 917812 Opened 6 years ago Closed 6 years ago

[Contacts] "Select All" should be "Select all" and buttons aren't vertically centered

Categories

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

All
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: amylee, Assigned: mai)

Details

(Whiteboard: visual design, visual-tracking, jian)

Attachments

(2 files, 1 obsolete file)

Attached image Contacts_FB.png
Hi, 

The text on the buttons for importing Facebook contacts should be vertically centered. Also, the "Select All" text should be "Select all".  

Thanks
Assignee: pivanov → wmathanaraj
blocking-b2g: hd? → ---
Assignee: wmathanaraj → gtorodelvalle
Assignee: gtorodelvalle → mri
Attached file patch v1 (obsolete) —
Attachment #813496 - Flags: review?(jmcf)
Flags: needinfo?(arnau)
Arnau,

Please could you confirm that the buttons are misaligned or not?

thanks
yes, text in buttons are 1px down, I'll have to move it BB.
Flags: needinfo?(arnau)
You mean BBs have to changed??
Flags: needinfo?(arnau)
Comment on attachment 813496 [details]
patch v1

please, change the commit message to reflect the bug number and the part you are fixing (Button Labels).
Attachment #813496 - Flags: review?(jmcf) → review+
Changed the commit message
Amy, I have reviewed the Building Block (confirm) and the code looks good to me:
height: 38px
line-height: 38px
That means the text should center vertically to the button AFAIK the 1px variation is due to the font baseline, which means I could fake the code to make the text fit, but in case of using another font with another baseline, could not be perfectly centered again.

So I would not fix that. WDYT?
Flags: needinfo?(arnau) → needinfo?(amylee.design)
Hi Arnau, 

As long as it's adjusted for Fira Sans I think this solution works. Also has the text in the button been corrected? "Select All" text should be "Select all".  

Thanks!

(In reply to Arnau March from comment #7)
> Amy, I have reviewed the Building Block (confirm) and the code looks good to
> me:
> height: 38px
> line-height: 38px
> That means the text should center vertically to the button AFAIK the 1px
> variation is due to the font baseline, which means I could fake the code to
> make the text fit, but in case of using another font with another baseline,
> could not be perfectly centered again.
> 
> So I would not fix that. WDYT?
Flags: needinfo?(amylee.design)
Attached file patch v2
Attachment #813496 - Attachment is obsolete: true
Attachment #817689 - Flags: review?(jmcf)
Amy,
After double checking this I found out there's a bug in the platform related with the default styles that affects all buttons, rendering them 1px down.
So I have created a PR fixing all Building Blocks which contained a button.
Thanks.
Whiteboard: visual design, visual-tracking → visual design, visual-tracking, jian
Comment on attachment 817689 [details]
patch v2

thanks Arnau
Attachment #817689 - Flags: review?(jmcf) → review+
https://github.com/mozilla-b2g/gaia/commit/71d9a0c80df5ad3d62a47531eb53651e077f8a6c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.