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

RESOLVED FIXED

Status

Firefox OS
Gaia::Contacts
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: amylee, Assigned: mai)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 806637 [details]
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
(Reporter)

Updated

4 years ago
Assignee: pivanov → wmathanaraj
(Reporter)

Updated

4 years ago
blocking-b2g: hd? → ---
status-b2g-v1.1hd: ? → ---

Updated

4 years ago
Assignee: wmathanaraj → gtorodelvalle

Updated

4 years ago
Assignee: gtorodelvalle → mri
(Assignee)

Comment 1

4 years ago
Created attachment 813496 [details]
patch v1
Attachment #813496 - Flags: review?(jmcf)

Updated

4 years ago
Flags: needinfo?(arnau)

Comment 2

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

Comment 4

4 years ago
You mean BBs have to changed??
Flags: needinfo?(arnau)

Comment 5

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

Comment 6

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

Comment 8

4 years ago
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)
Created attachment 817689 [details]
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.

Updated

4 years ago
Whiteboard: visual design, visual-tracking → visual design, visual-tracking, jian

Comment 11

4 years ago
Comment on attachment 817689 [details]
patch v2

thanks Arnau
Attachment #817689 - Flags: review?(jmcf) → review+

Comment 12

4 years ago
https://github.com/mozilla-b2g/gaia/commit/71d9a0c80df5ad3d62a47531eb53651e077f8a6c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.