210.76 KB, image/jpeg
204.97 KB, image/jpeg
302.98 KB, image/png
46 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
5.17 MB, video/mp4
Created attachment 808431 [details] On device #1 STR: 1. Lock the phone by pressing the power button 2. The lockscreen shows 3. Pull one of the handles to left/right to unlock Excepted: The slider will be full of the blue color. Actual Result: There're one or two glitches among the slider, and they will be invisible after few seconds even when user's finger is still at the same position. Notes: 1. It's only obvious on real devices. On FirefoxNightly for Mac & Linux, the glitches gone or become very fuzzy. 2. Attachments are the screenshots.
Summary: Fix the glitches among the sliders when user is pulling the handle → [LockScreen] Fix the glitches among the sliders when user is pulling the handle
Well, the old hack tricks work here: using a scaling factor to scale the center area to overlap on the possible glitches eases the headache. However, there're still some sub-pixel lines show near the center if user pull the handle close to it. I shall investigate what exactly happens here.
I've pushed the WIP patch to GitHub, here is the link to the branch: https://github.com/snowmantw/gaia/tree/issue919410 But the problem wasn't solved completely. There're still some thin lines will appear while the handle is bouncing.
I will check the slider components' screen position first.
A debug version to show the detail of the elements: https://github.com/snowmantw/gaia/tree/issue919410
Jerry: or I put the original slider beyond the half-opacity area and seperate the elements in it, then we can get rid off the debug sliders?
(In reply to Greg Weng [:snowmantw] from comment #7) > Jerry: or I put the original slider beyond the half-opacity area and > seperate the elements in it, then we can get rid off the debug sliders? I've make a new debug version as I said and pushed to the branch above.
Greg will provide bandage fix for v1.2 in bug 921360, set this bug to -'ing.
blocking-b2g: koi+ → -
I'll remove some 'rem' is CSS and fix them with static pixel width, to make sure the whole sliding component can be the same at every device. Rest parts of the lockscreen can remain the 'rem' as the unit.
In git commit d34bdb0aada2f847ba6f3e9fa8457b4dbee128e2. I print some position info for slider. In unagi device, the rectangle and the right half circle are at: upper left(159.599976,400.000000), lower right(190.399975,460.000000) upper left(189.000000,400.000000), lower right(219.000000,460.000000) In leo device, , the rectangle and the right half circle are at: upper left(160.000000,400.000000), lower right(194.000000,460.000000) upper left(192.000000,400.000000), lower right(222.000000,460.000000) The component seems to be overlapped with a little pixel width and you will see some bright line at the overlapped area when you pull the slider bar.
I'm trying to use canvas to solve this problem completely. So far it works perfectly. The jsFiddle: http://jsfiddle.net/jxjY7/
Created attachment 832063 [details] WIP patch I keep legacy code to make sure there're nothing would be broken due to my patch. These code would be removed in the progress of following refactoring. Some different points (some of them would be fixed soon): 1. The gradient color happens on sliding, and not according to the fixed timing but the distance user dragged. 2. The arrows now slide incorrectly, because they are affected by the legacy parts. I would try to pull them out and use new method to implement it. 3. The glitches had gone and the performance is still acceptable. Thanks God. It also should work on larger devices (tested on FirefoxNightly). BTW, I keep the commits because this is just a WIP patch. I'll squash them all when this patch can be landed. --- Future works 1. Fix bugs 2. Move it into a standalone app at Bug 898348 3. Integrate it with incoming call mode 4. Bug 937442 - Implement LockWindow to manage LockScreen
Summary: [LockScreen] Fix the glitches among the sliders when user is pulling the handle → [LockScreen] Re-implement LockScreen in Canvas to solve the glitches issue
Change the title because we need to focus on the Canvas implementation.
Created attachment 832778 [details] [review] Patch This is the Canvas version of the LockScreen. It eliminates the glitches problem. A particular change is the unlock handle now can only be triggered by touching on the circle, which was described in the original spec at the Bug 887802. I also improved the auto-expanding of the handle. It now uses acceleration instead of automatically pull-to-end. The last change is the opacity now changing according to the length of sliding, not a fixed time, although their effects are almost the same. This involved some technique difficulties so far I can't solve. If it's possible, UX may review this too see if we need to fire another bug to use the old way, or the new stlye is okay.
I'm now do some adjustments about pixel ratio. So the patch will be updated with some WIP commits. I'll squash them again once the issue is resolved.
blocking-b2g: - → 1.3+
Comment on attachment 832778 [details] [review] Patch Discussed offline
Solved several pixel mapping issues today. The only one left is the image issue.
Created attachment 8335809 [details] [review] Patch New patch. Refactored as follow: 1. Move unlocking code to new, independent unlocking strategy named LockScreenSlide. 2. The LockScreen would need to composite the strategy to unlock itself 3. There would be a intentionRouter sent by LockScreen to the unlocker, to let it can communicate with the LockScreen 4. In this way, it's possible to change or add new unlocker without modify the LockScreen itself. The LockScreen just need to be initialized with the new strategy 5. Other functions are still inside the LockScreen, because they're irrelevant to the unlocker.
Comment on attachment 8335809 [details] [review] Patch Well rebase on the newest master would make it doesn't appear even turn on the setting. I need to check it first.
Fabrice raised an issue with this 'bandaging' on the other bug https://bugzilla.mozilla.org/show_bug.cgi?id=898348#c18 I'd like to know the answer to that too. I'd like to know if a fix for the underlying problem was considered and if there was a discussion with the Graphics team. It might be faster/easier/cheaper to fix that.
While canvas is cool, there is no longer a DOM tree to manipulate, which defeats the point of a HTML based phone.
(In reply to Nikhil Marathe [:nsm] (use needinfo?) from comment #22) > While canvas is cool, there is no longer a DOM tree to manipulate, which > defeats the point of a HTML based phone. IMO if you really implement an animation, you should use Canvas, SVG and even WebGL. What happens on us is the similar question. It's not a shame to use this techniques to implement animations, but using DOMs and CSSs may be (remember those "3D" animations before WebGL?)
Comment on attachment 8335809 [details] [review] Patch Solved the problem and sent it again.
Travis was green: https://travis-ci.org/mozilla-b2g/gaia/builds/14297593
(In reply to Nikhil Marathe [:nsm] (use needinfo?) from comment #22) > While canvas is cool, there is no longer a DOM tree to manipulate, which > defeats the point of a HTML based phone. I don't know. Maybe 10 years from now we will stop writing JS/DOM/CSS and all websites will be run with compiled asm.js and WebGL. But I know converting our lock screen handle to canvas does not accelerate nor slow the trend, while solving our problems. :)
Update: I was caught up by 3rd-party keyboard wrapping up for v1.2 so this will have to wait for a few more day for review. Sorry about that.
Comment on attachment 8335809 [details] [review] Patch Everything is good except tests and API between LockScreenHandle and LockScreen.
Attachment #8335809 - Flags: review?(timdream) → feedback+
Comment on attachment 8335809 [details] [review] Patch We would like to have some UI and perceptional performance review on the implementation.
Attachment #8335809 - Flags: ui-review?(firefoxos-ux-bugzilla)
Comment on attachment 8335809 [details] [review] Patch Assigning UX review from general UX to Jaime and adding a note to make sure we all review this in our first pre-FC UX-QA Quality pass at 4 PM PST today.
Attachment #8335809 - Flags: ui-review?(firefoxos-ux-bugzilla) → ui-review?(jachen)
And since I don't know if I disable the two icons from the clicking event, whether the ScreenReader would still work or not, I filed another bug to discuss this: Bug 943237
Comment on attachment 8335809 [details] [review] Patch Add lockscreen's tests and change flag to review.
Attachment #8335809 - Flags: feedback+ → review?(timdream)
Attachment #8335809 - Flags: review?(timdream) → review+
Travis was green: https://travis-ci.org/mozilla-b2g/gaia/builds/14682088
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8335809 [details] [review] Patch We reviewed this today and the lockscreen does not work with incoming calls. Rob will post a vidyo shortly.
Attachment #8335809 - Flags: ui-review?(jachen) → ui-review-
In our review today, we discovered a number of issues with the incoming call lock screen as well as the lock screen. I will be filing bugs and meeting with Greg and Steve in the Taipei office tomorrow. The video that Jaime references for the incoming call unlock bug is referenced in Bug 945715 comment 1.
Hi, Greg, Thanks for your help! I verified this bug. * Test Build: - Gaia: c952e2756c03eceb4de6a3eba15651741a62f9e8 - Gecko: http://hg.mozilla.org/mozilla-central/rev/df82be9d89a5 - BuildID 20131210040206 - Version 29.0a1 Attaching the verification result. FYI.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.