1187979 - Improve Lockscreen Inputpad's touch and visual feeback

Status: RESOLVED FIXED

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

Description

[Christiane Ruetten [INACTIVE]]

Inputpad looks and feels wonky compared to the slick lock and feel of
the keyboard panel used while setting the pass code in Settings. Much of
the reason comes from that it only vibrates on key release, the other
that it does not behave well when the user is sliding a finger over the
keys. There is also a considerable delay between touching a key and a
visual reaction to the extent that it's skipped when typing mildly fast.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] cr:lockscreentouch > mozilla-b2g:master

Comment 2

[Christiane Ruetten [INACTIVE]]

The prototype patch addresses all of the above:

- make InputPad vibrate instantly on touch
- make InputPad respect user's touchmove
- visualize key presses instantly

It currently breaks the vibration test, but it's only meant to start discussion anyway.

Let me know what you think.

Comment 3

[Christiane Ruetten [INACTIVE]]

The visualization delay appears to be scaling according to hardware performance. While really irritating on the Flame, it's barely noticeable on the much more powerful foxfood device.

Comment 4

[Tiffanie Shakespeare [:tif]]

Comment on attachment 8639355 [details] [review]
[gaia] cr:lockscreentouch > mozilla-b2g:master

Adding ui-review for Harly (interaction) and Patryk (visual).

Thanks for pinging the UX team!

Comment 5

[Tiffanie Shakespeare [:tif]]

oops forgot to remove our flag.

Comment 6

[Harly Hsu[:harly]]

Hi Christiane, 
After taking a look at the patch, I can see that you have made the change of having the phone vibrate immediately instead of after release when user presses the keypad. However, I don't really feel a difference for "make InputPad respect user's touchmove" and "visualize key presses instantly", could you be more specific as what does these 2 thing do? 

Also, it will be great if we can add a small feature which is when user long presses the back key, system will delete all the entered pass code like what we have in the pass code settings. Thanks!

Comment 7

[Christiane Ruetten [INACTIVE]]

Hi Harly,

 for me it makes a difference on which hardware I test. Generally the "wonkiness" feels stronger on the Flame than on the beefier Foxfood device. The visualization delay could be detected on the Flame only. There was a ~200ms delay between touching a key and the key being visualized as "active".

For the touch-slide issue, touch your finger down on a key, slide around and release over a different key. I expect to see the "active" key move around with my finger, and that I input the key over which I release just like the normal keyboard does.

I tested this on several devices and b2g versions, so I am not able to exactly reproduce from memory, but what I observed were issues like:

- "active" key not following finger movement, stuck on the first key
- no key or wrong key being input when sliding to a different key

Does that make it clearer?

I like the idea of the "clear pin" feature and it is easy to implement. What's the regular delay for holding keys?

Comment 8

[Harly Hsu[:harly]]

Comment on attachment 8639355 [details] [review]
[gaia] cr:lockscreentouch > mozilla-b2g:master

Thanks for clarifying, I think the patch has all of the things you've mentioned and it is working nicely. About the long press back key, could you reference the code from the passcode settings, I don't really know the time delay for long press.

Comment 9

[Patryk Adamczyk [:patryk] UX]

Comment on attachment 8639355 [details] [review]
[gaia] cr:lockscreentouch > mozilla-b2g:master

Works well.

Comment 10

[Christiane Ruetten [INACTIVE]]

Comment on attachment 8639355 [details] [review]
[gaia] cr:lockscreentouch > mozilla-b2g:master

Greg, it looks like UX likes the change. Rebased to today's master, local tests passing. I think it's ready for final review.

Comment 11

[Christiane Ruetten [INACTIVE]]

Finalized the code according to gweng's comments on the pull request. please r+.

Comment 12

[Greg Weng [:snowmantw][:gweng][:λ]]

It's great work and much more clear now. But would you mind to put the event handling statements into different functions, like `onTouchStart(evt)` and call that in the switch branches? I usually do that when the switch grows up like this, since to have a long switch case is bad for tracing and reading. Moreover, it makes unit tests much clear (since we only need to test each function, rather than test the same function with the switch). After that, you may also add some tests for each handler, which also can guarantee individual functions work well in the future.

Comment 13

[Christiane Ruetten [INACTIVE]]

Greg, I started out with creating separate event sub-handlers, but then was too appalled by blatant code duplication and lack state compartmentalization. I ended up with a major overhaul of InputPad and the accompanying test suite. I hope it now has the clarity as well the as code coverage that it deserves. 

Please re-review and r+ if you agree.

Comment 14

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8639355 [details] [review]
[gaia] cr:lockscreentouch > mozilla-b2g:master

Thanks. The code now looks really great. Let's see if it works well on daily use.

Comment 15

[Christiane Ruetten [INACTIVE]]

Thanks, Greg.

Comment 16

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/f64408f42bd81f2742f2b48725240016660ffe39