1053613 - Lockscreen PIN keypad: backspace key misaligned & wrong, vertical separators missing, last row too high

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)

Description

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: 2014-08-14-10-45-32.png

As illustrated in the attached screenshot, the backspace key of lockscreen PIN keypad is right aligned. It should be center aligned.

Comment 1

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: Patch (PR @ GitHub)

Hi Omega, please check if it's correctly aligned now. Thanks!

Comment 2

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8472771 [details] [review]
Patch (PR @ GitHub)

I think Carol is the better reviewer for this bug.
BTW, I found another issue. The PIN code keyboard in lockscreen lacks of the vertical separators between keys.

Comment 3

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8472771 [details] [review]
Patch (PR @ GitHub)

(In reply to Omega Feng [:Omega] [:馮於懋] from comment #2)
> BTW, I found another issue. The PIN code keyboard in lockscreen lacks of the
> vertical separators between keys.

Ah, I will fix that in this bug too. Canceling the review request for now.

Comment 4

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8472771 [details] [review]
Patch (PR @ GitHub)

Hi Carrol,

Both the misalignment and the missing separators have been fixed. Please check it out. Thanks a lot!

Comment 5

[Carol Huang [:Carol]]

Comment on attachment 8472771 [details] [review]
Patch (PR @ GitHub)

Hi John,
I've checked the patch and there are two things:

1. the keypad height of bottom row
2. the backspace-key 

I would like them visually the same as our Pin code pad. Thank you!

Comment 6

[Carol Huang [:Carol]]

Attachment: Bug1053613_Lockscreen PIN keypad.jpg

Hi John,
I'll just upload a attachment to let you know what I was talking about according to the UI review. Thanks!

Comment 7

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: Patch again (PR @ GitHub)

Hi Carol: how does this patch look? Thanks!

Comment 8

[Carol Huang [:Carol]]

Comment on attachment 8472899 [details] [review]
Patch again (PR @ GitHub)

Hi John,
Could you remove the divider on the very right side?
I'll attach a screenshot.
Thanks!!

Comment 9

[Carol Huang [:Carol]]

Attachment: patch_screenshot_0815.jpg

please remove the divider on the right side of the keyboard.

Comment 10

[John Lu [:mnjul][Please NI or I'll miss]]

(In reply to Carol Huang [:Carol] from comment #9)
> Created attachment 8473551 [details]
> patch_screenshot_0815.jpg
> 
> please remove the divider on the right side of the keyboard.

Fixed that. Could you check the patch again? Thanks.

Let's finalize the ui-review after bug 1054179 is resolved.

Comment 11

[Carol Huang [:Carol]]

It looks good!
Let's do a UI review again when bug 1054179 is resolved. Thanks!!

Comment 12

[John Lu [:mnjul][Please NI or I'll miss]]

HI Carol,

bug 1054179 would be done after FL, but this current bug should be low-risk enough to land before FL. I suggest that we finish this bug without incorporating/waiting for changes of bug 1054179, such that the two numpads would look identical at FL. I will file a follow-up bug for lockscreen's numpad rows' even-distributed heights.

How does this sound? Thanks.

Comment 13

[John Lu [:mnjul][Please NI or I'll miss]]

NI Carol for comment 12 ^^^

Comment 14

[Carol Huang [:Carol]]

Hi John,
sounds good! let's do this. 
Please cc me the follow-up bug.
Thank you!!

Comment 15

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8472899 [details] [review]
Patch again (PR @ GitHub)

(In reply to Carol Huang [:Carol] from comment #14)
> Hi John,
> sounds good! let's do this. 
> Please cc me the follow-up bug.
> Thank you!!

Hi Carol, it would be great if we have ui-review+ (according to the criteria of: (1) backspace key misalignment & appearance (2) vertical separators; not including the evenly-distributed heights) before landing the patch. Thanks!

Comment 16

[Carol Huang [:Carol]]

Comment on attachment 8472899 [details] [review]
Patch again (PR @ GitHub)

Hi John,
The UI looks good! thank you!

Comment 17

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8472899 [details] [review]
Patch again (PR @ GitHub)

Hi Greg, here's a CSS patch for you to review. Thanks!

Tim, asking for your feedback regarding moving Keyboard-Symbols.woff.
The story: the backspace key that concerns UX in comment 5 is that (as offline discussed) in lockscreen it looks different from how it looks in other places. I discovered this was due to missing font file. Note that keyboard app hasn't actually been using Keyboard-Symbols.woff since bug 992346, so in my patch I moved the file to system app instead of moving into /shared. Does it sound okay?

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8472899 [details] [review]
Patch again (PR @ GitHub)

Let's use what's done in bug 1030829 and bug 1008458 to "load" these fonts from both apps, thanks.

Comment 19

[Tim Guan-tin Chien [:timdream] (please needinfo)]

... to load the Keyboard Symbols font from both apps :-/

Comment 20

[John Lu [:mnjul][Please NI or I'll miss]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #18)
> Comment on attachment 8472899 [details] [review]
> Patch again (PR @ GitHub)
> 
> Let's use what's done in bug 1030829 and bug 1008458 to "load" these fonts
> from both apps, thanks.

Doing that in 1056514.

Comment 21

[John Lu [:mnjul][Please NI or I'll miss]]

Bug 1056514 is now resolved, will modify this bug accordingly to land and open follow-up as agreed with Carol in comment 15.

Comment 22

[John Lu [:mnjul][Please NI or I'll miss]]

Testing at https://tbpl.mozilla.org/?rev=b3c5359e9651a8cd3d0ce200e50b4a5cfa17e01b&tree=Gaia-Try in preparation for landing.

Comment 23

[John Lu [:mnjul][Please NI or I'll miss]]

Master: https://github.com/mozilla-b2g/gaia/commit/c8e4549c4e73606a0f50ee751b165b1f92bf8924

Comment 24

[John Lu [:mnjul][Please NI or I'll miss]]

(In reply to John Lu [:mnjul] [MoCoTPE] from comment #12)
> HI Carol,
> 
> bug 1054179 would be done after FL, but this current bug should be low-risk
> enough to land before FL. I suggest that we finish this bug without
> incorporating/waiting for changes of bug 1054179, such that the two numpads
> would look identical at FL. I will file a follow-up bug for lockscreen's
> numpad rows' even-distributed heights.
> 
> How does this sound? Thanks.

This is now in bug 1059075.

Comment 25

[Francesco Lodolo [:flod]]

Have you tried with other languages? We have text on three lines for some locales, and the effect is horrible
http://transvision.mozfr.org/string/?entity=apps/system/system.properties:emergency-call-button&repo=gaia

I honestly think this should be backed out and put more thought into it.

Comment 26

[Francesco Lodolo [:flod]]

Attachment: Screenshot of flame set to Italian

Comment 27

[John Lu [:mnjul][Please NI or I'll miss]]

Many thanks Francesco. I'll talk with our UX for this.

Omega & Carol, could you take a look at this? I kinda think we might still need to have a somewhat higher-than-others last row to accommodate the three lines in other languages.

Comment 28

[Carol Huang [:Carol]]

Hi John, 
please do the revise below:
1. make the text size 0.2 rem 'smaller'
2. the text should be not only horizontally center aligned in the field but also 'Vertically'. So the text won't be cropped on the bottom.

Thank you!!

Comment 29

[Francesco Lodolo [:flod]]

One other check: try with a very long first word (of with the pseudolocale Accented English). I think you don't have a max-width set, so the horizontal alignment could go off axis.

Comment 30

[John Lu [:mnjul][Please NI or I'll miss]]

FYI, the "Emergency Call" text thing will be dealt with in bug 1060264.

Comment 31

[Omega Feng [:Omega] [:馮於懋]]

We fixed the Emergency Call problem in bug 1060264. Good job, thanks!