Closed Bug 1187979 Opened 7 years ago Closed 7 years ago

Improve Lockscreen Inputpad's touch and visual feeback


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

Gonk (Firefox OS)
Not set


(b2g-master fixed)

FxOS-S7 (18Sep)
Tracking Status
b2g-master --- fixed


(Reporter: cr, Assigned: cr)



(Keywords: feature)


(1 file)

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.
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.
Flags: needinfo?(firefoxos-ux-bugzilla)
Summary: Improve Inputpad's touch and visual feeback → Improve Lockscreen Inputpad's touch and visual feeback
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 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!
Attachment #8639355 - Flags: ui-review?(padamczyk)
Attachment #8639355 - Flags: ui-review?(hhsu)
oops forgot to remove our flag.
Flags: needinfo?(firefoxos-ux-bugzilla)
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!
Flags: needinfo?(cr)
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?
Flags: needinfo?(cr) → needinfo?(hhsu)
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.
Flags: needinfo?(hhsu)
Attachment #8639355 - Flags: ui-review?(hhsu) → ui-review+
Comment on attachment 8639355 [details] [review]
[gaia] cr:lockscreentouch > mozilla-b2g:master

Works well.
Attachment #8639355 - Flags: ui-review?(padamczyk) → ui-review+
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.
Attachment #8639355 - Flags: review?(gweng)
Finalized the code according to gweng's comments on the pull request. please r+.
Flags: needinfo?(gweng)
Assignee: nobody → cr
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.
Flags: needinfo?(gweng) → needinfo?(cr)
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.
Flags: needinfo?(cr) → needinfo?(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.
Flags: needinfo?(gweng)
Attachment #8639355 - Flags: review?(gweng) → review+
Thanks, Greg.
Keywords: checkin-needed
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S7 (18Sep)
Depends on: 1203269
You need to log in before you can comment on or make changes to this bug.