What happens ------------ Launch FM Radio app. Drag dial. There is a high latency between touch and dial response. Occasionally, frames are dropped during animation. Why --- Notches in FM Radio app dial are currently rendered as floated divs, making for lots of DOM elements and lots of layout. Proposed solution ----------------- Notches have a static width/height and distance between. Currently, the DOM elements are used purely for visual reasons. Use a background image instead of separate DOM elements for every notch.

Pull request (fixes bug): https://github.com/mozilla-b2g/gaia/pull/6224

NOTE: If blocking-basecamp+ is set, just land it for now. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: radio tuner dial does not consistently respond to user input. Testing completed: Risk to taking this patch (and alternatives if risky): low. I did the simplest possible thing that can work. Speaking with Chris Jones, we could also take an alternative approach that would further simplify the code, but would increase the amount of code changed.

Comment on attachment 679293 [details] [diff] [review] Patch for issue 809294: improve speed of radio tuner dial (generated via git diff) Review of attachment 679293 [details] [diff] [review]: ----------------------------------------------------------------- ::: apps/fm/js/fm.js @@ +330,5 @@ > var dialValue = markStart + i * 0.1; > + if (dialValue < start) { > + startMaskWidth += unitWidth; > + } > + else if (dialValue > end) { Should "else" be the same line as the last curly bracket, like this: } else if (dialValue > end) { ? ::: apps/fm/style/fm.css @@ +124,4 @@ > font-weight: lighter; > height: 9rem; > overflow: hidden; > + background: #4C5055; The linear gradient background color and the decoration line under the dialer are lost, please check the visual design: https://wiki.mozilla.org/Gaia/FMRadio#Visual

Hi Pin Zhang! Thanks for your feedback. > Should "else" be the same line as the last curly bracket, like this: > } else if (dialValue > end) { > ? Either approach is valid. Happy to change this if our style guide prefers same line. > The linear gradient background color and the decoration line under the dialer are lost, please check the visual design: https://wiki.mozilla.org/Gaia/FMRadio#Visual Thanks for noticing. This came out of a chat with Patryk of visual design. With the low color-depth of the phone screen, the gradient doesn't really make a visual difference. Patryk agreed we were better off without it. Patryk, can I get you to confirm? Thanks!

Yes Pin, Gordon and I talked about this changed, please approve it.

if { } else if { } And you want to request a new review :)

(In reply to Patryk Adamczyk [:patryk] UX from comment #5) > Yes Pin, Gordon and I talked about this changed, please approve it. So the last DIV child node of the "dialer-container" and the related styles should be removed, please update the commits of the github issue #6224, r=me

Hi Pin, I've updated the pull request with the change you requested. Currently pull request is issued as a series of commits. As a previous user of hg, git rebase is bending my mind a bit. LMK if a rebase is a must. Thanks!

Comment on attachment 679293 [details] [diff] [review] Patch for issue 809294: improve speed of radio tuner dial (generated via git diff) (In reply to Gordon Brander :gbrander from comment #8) > Hi Pin, > I've updated the pull request with the change you requested. > > Currently pull request is issued as a series of commits. As a previous user > of hg, git rebase is bending my mind a bit. LMK if a rebase is a must. > Thanks! Let's reask him for review then. It is sometimes hard to see a comment in bugmails.

Comment on attachment 679293 [details] [diff] [review] Patch for issue 809294: improve speed of radio tuner dial (generated via git diff) I forgot the name of the reviewer in the field. Sorry for the spam.

(In reply to Gordon Brander :gbrander from comment #8) > Hi Pin, > I've updated the pull request with the change you requested. > > Currently pull request is issued as a series of commits. As a previous user > of hg, git rebase is bending my mind a bit. LMK if a rebase is a must. > Thanks! r=me To keep the commit clean, I think you should rebase your pull request (#6224), the merge commits which modified the camera app doesn't make any sense in this bug fixing.

Comment on attachment 679293 [details] [diff] [review] Patch for issue 809294: improve speed of radio tuner dial (generated via git diff) Review of attachment 679293 [details] [diff] [review]: ----------------------------------------------------------------- Carrying pzhang r+. Make sure to address his comments and rebase your commits :)

Verified in 11/27/2012 build Unagi phone, build 2012/11/27