Closed Bug 917812 Opened 11 years ago Closed 11 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
normal

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+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: