Closed
Bug 1118006
Opened 9 years ago
Closed 9 years ago
In Edit Mode, long press (to initiate move group or move app icon) is too long to trigger action, causes user confusion
Categories
(Firefox OS Graveyard :: Gaia::Homescreen, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: kcaldwell, Assigned: cwiiis)
References
Details
(Keywords: polish, Whiteboard: [systemsfe], [ux-most-wanted])
Attachments
(2 files, 1 obsolete file)
46 bytes,
text/x-github-pull-request
|
kgrandon
:
review+
bajaj
:
approval-gaia-v2.2+
|
Details | Review |
5.70 MB,
video/mp4
|
Details |
In edit mode, long press (to trigger move group or app) 'feels' too long and causes the user to think that interaction doesn't work. [During informal user testing, when asked to create a new group for a specific app, participant was frustrated by the unresponsive press and hold on the icon, that they gave up and assumed they were doing something wrong.] Recommendation: Shorten 'long press' in Edit Mode
please also see Bug 1092325 - Remove need for long press when moving icons/groups in edit mode (edit)
Assignee | ||
Comment 2•9 years ago
|
||
Removing app-grouping tag, this bug exists regardless of app-grouping.
Summary: (App-Grouping) In Edit Mode, long press (to initiate move group or move app icon) is too long to trigger action, causes user confusion → In Edit Mode, long press (to initiate move group or move app icon) is too long to trigger action, causes user confusion
Updated•9 years ago
|
Whiteboard: [systemsfe]
Assignee | ||
Comment 3•9 years ago
|
||
This is about the best I could do in short order. This is already a reasonably risky patch I think, anything better would require more invasive changes (probably - I'm sure someone talented could come and prove me wrong :)). This branch shortens the delay to 100ms, which is about as short as you can make it without making it really easy to trigger accidentally, even in edit mode. The delay feels longer than that because a lot of work is triggered when you start dragging an icon and the rendering takes quite a long time. A better patch would involve reworking things so some of that work happens in a more delayed fashion. Katie, what do you think? https://github.com/Cwiiis/gaia/tree/bug1118006-shorter-edit-mode-longpress
Flags: needinfo?(kcaldwell)
not sure if it's me or my phone... but when I loaded the patch, when I enter edit mode (long press on collapsed group on homescreen), enter edit mode, collapsed group highlights, but gets 'stuck' and I can no longer move it. Done and home button no longer work. only way to exit, is to power off. tried reflashing my phone, reloading the patch. happened twice. let me know when you want to try again, as I can't test whether or not the 'long press' feels shorter. :)
Flags: needinfo?(kcaldwell)
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to katieC from comment #4) > not sure if it's me or my phone... but when I loaded the patch, when I enter > edit mode (long press on collapsed group on homescreen), enter edit mode, > collapsed group highlights, but gets 'stuck' and I can no longer move it. > Done and home button no longer work. only way to exit, is to power off. > > tried reflashing my phone, reloading the patch. happened twice. > > let me know when you want to try again, as I can't test whether or not the > 'long press' feels shorter. :) Some rebasing broke it, fixing now - but running into some other things that I'd like to fix before pushing again...
Assignee | ||
Comment 6•9 years ago
|
||
Ok, try now :) I've got quite a nice performance optimisation in there that I think we may want to try and include aside from the shorter long-press time. Note, the shorter long-press is only for *after* you've entered edit mode. I find that sometimes initiating a long-press immediately after finishing another one doesn't work because there's a pretty long time after you let go of an icon where animations are running and the page is re-rendering... There are probably some things we can do to help that, but see what you think of this first (this is already getting to be a bit more invasive than I think is a good idea at this stage. Not quite, but almost).
Flags: needinfo?(kcaldwell)
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•9 years ago
|
||
Updated branch to latest master, which contains all current app-grouping fixes.
Assignee | ||
Comment 8•9 years ago
|
||
Looking for some feedback on this approach - especially if there's something more obvious that I've missed. Something I don't like in this patch is the modification to verticalhome - it would be nicer to be able to encapsulate this in the element somehow, but I don't think we can do that without making some bad assumptions. Currently, the element is the size of the entire grid and the embedder controls scrolling - so if you want to disable overflow without altering the scroll position, it's the containing element whose style needs to change. We could look up the container in the GaiaGrid JS and override the style, I suppose, but I fear it'd break some possible use-cases... Am I being over-cautious? Wrt the overflow being disabled, this is required or you can end up in a state where you're dragging the icon and scrolling (even when you preventDefault on the touchmove events, which strikes me as unintuitive) - but it also has the nice side effect of massively boosting performance. It might be an idea to incorporate the overflow:hidden hack as a performance boost elsewhere, but we'd need to be careful not to do it in ways that could interrupt when a user wants to scroll.
Attachment #8545922 -
Flags: feedback?(kgrandon)
Attachment #8545922 -
Flags: feedback?(crdlc)
Comment 9•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode I think the approach makes sense to me.
Attachment #8545922 -
Flags: feedback?(kgrandon) → feedback+
Comment 10•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode The idea makes sense to me too. Left a couple of comments but not significant
Attachment #8545922 -
Flags: feedback?(crdlc) → feedback+
Assignee | ||
Updated•9 years ago
|
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → All
Reporter | ||
Comment 11•9 years ago
|
||
Hey Chris, this shortened 'long-press' patch feels much more responsive in edit mode.
Flags: needinfo?(kcaldwell)
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Tiffanie Shakespeare from comment #12) > What needs to happen for this to land? Really, just some extensive testing and then review. Last time I tried testing this extensively, I was able to get it to do something weird, but a lot has changed around it now, so it needs re-testing. I'll get to it soon-ish.
Flags: needinfo?(chrislord.net)
Assignee | ||
Comment 14•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode I think this works alright now. I've rebased it on master and done a reasonable amount of testing and worked out various issues. (that said, this is definitely a patch that needs to bake for at least a week before uplift)
Attachment #8545922 -
Flags: review?(kgrandon)
Attachment #8545922 -
Flags: review?(crdlc)
Comment 15•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode This is looking pretty good. I left a few comments on github that I'd like to see addressed if possible. Please re-flag after addressed.
Attachment #8545922 -
Flags: review?(kgrandon)
Comment 16•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode I am fine with the code and Kevin's suggestions, please take a look at them. Good job as always
Attachment #8545922 -
Flags: review?(crdlc)
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode Ready for another round I think - I resolved issues with tapping and multi-touch, and tried to address your comments (I've answered everything in github where appropriate).
Attachment #8545922 -
Flags: review?(kgrandon)
Comment 18•9 years ago
|
||
I'm pretty happy with this, but I'd like to get a better understanding of where the tests are before leaving my R+. The last gaia-try run seemed quite red due to infra issues, but there is at least one real failure in Gij7. I've closed/re-opened the pull request to try to re-trigger gaia-try. Chris - can you take a look at the tests? Here is a link to one of the real failures: http://ftp.mozilla.org/pub/mozilla.org/b2g/try-builds/gaiabld-fa6b3db56505/gaia-try-linux64_gecko/gaia-try_ubuntu64_vm-b2gdt_test-gaia-js-integration-9-bm116-tests1-linux64-build33.txt.gz
Flags: needinfo?(chrislord.net)
Comment 19•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode Just clearing my queue a bit. Let's see what these tests do. Assuming they pass and there's not a lot more changes required then I'm sure I could give this an R+ quickly.
Attachment #8545922 -
Flags: review?(kgrandon)
Assignee | ||
Comment 20•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode ok, all the tests are looking good now :) (the marionette failures were legit due to a mistake I made rebasing, the unit test just needed a clock tick).
Flags: needinfo?(chrislord.net)
Attachment #8545922 -
Flags: review?(kgrandon)
Comment 21•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode Left a small nit about the added line, but r=me. Nice work.
Attachment #8545922 -
Flags: review?(kgrandon) → review+
Assignee | ||
Comment 22•9 years ago
|
||
nit addressed. This is definitely a patch we want to let bake for a few days.
Keywords: checkin-needed
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 24•9 years ago
|
||
Just duped a blocker to this, so requesting blocking status. If we have to back this out, it isn't too much hassle to separate the patch for the blocking issue to land that separately.
blocking-b2g: --- → 2.2?
Comment 25•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/6d715904c0acf41612797a53dff455f23cdd049b
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 26•9 years ago
|
||
Reverted for causing an intermittent in Gij7, that started when this landed: https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=bf42f5be8eaf http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/b2g-inbound-linux64_gecko/1425064828/b2g-inbound_ubuntu64_vm-b2gdt_test-gaia-js-integration-7-bm53-tests1-linux64-build17.txt.gz https://github.com/mozilla-b2g/gaia/commit/c8ed1085a67490a1ecd7f275e5de9487e1b93b1d Chris - can you take a look? This was reproducing pretty constantly for me locally, let me know if you need help debugging it. The icon seems to get stuck in a "large" state, so I'm not sure if it's a problem with the test, or we're missing some touchend event. Let's try to run that test a bunch before re-landing.
Status: RESOLVED → REOPENED
Flags: needinfo?(chrislord.net)
Resolution: FIXED → ---
Assignee | ||
Comment 28•9 years ago
|
||
I think the failure may just be it doesn't wait for animations to finish before pressing the remove button... Will see if I can introduce a better fix than 'up the wait time', but otherwise I'll go with that.
Assignee | ||
Comment 29•9 years ago
|
||
Confirmed, just adding a .wait(1) after the long-press fixes this - I'm not sure exactly what it is about my patch that affected this test, possibly it's more stringent about not starting a drag before the state of the previous drag has been fully reset, possibly it's the wait on touchend before calling finalize. Going to see if I can alter the test slightly to fix this without introducing a wait.
Comment 30•9 years ago
|
||
Assignee | ||
Comment 31•9 years ago
|
||
Comment on attachment 8571891 [details] [review] [gaia] Cwiiis:bug1118006-shorter-edit-mode-longpress > mozilla-b2g:master This does the trick, but I'll wait on your ok to mark check-in.
Attachment #8571891 -
Flags: review?(kgrandon)
Updated•9 years ago
|
blocking-b2g: 2.2? → 2.2+
Comment 32•9 years ago
|
||
Comment on attachment 8571891 [details] [review] [gaia] Cwiiis:bug1118006-shorter-edit-mode-longpress > mozilla-b2g:master Test changes look good, and I've retirggered Gij7 quite a bunch and they've all passed. Thanks for taking care of it.
Attachment #8571891 -
Flags: review?(kgrandon) → review+
Comment 33•9 years ago
|
||
In master: https://github.com/mozilla-b2g/gaia/commit/ca977c05017e118a2a2ebefe2bd86a69cf968324
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Comment 34•9 years ago
|
||
Comment on attachment 8545922 [details] [review] Implement a shorter long-press delay in edit mode Obsoleting the old patch.
Attachment #8545922 -
Attachment is obsolete: true
Comment 35•9 years ago
|
||
Please request Gaia v2.2 approval on this patch when you get a chance.
status-b2g-v2.2:
--- → affected
status-b2g-master:
--- → fixed
Flags: needinfo?(chrislord.net)
Target Milestone: --- → 2.2 S7 (6mar)
Assignee | ||
Comment 36•9 years ago
|
||
Comment on attachment 8571891 [details] [review] [gaia] Cwiiis:bug1118006-shorter-edit-mode-longpress > mozilla-b2g:master Ok, no one's reported any fall-out from this yet, so I guess it's ok. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): [User impact] if declined: Icon drags take a long time to initiate, causes frustration/confusion [Testing completed]: On master for a couple of weeks, tested manually, homescreen is covered by lots of automated tests (both unit and marionette) [Risk to taking this patch] (and alternatives if risky): Medium risk of causing unexpected behaviour when dragging icons or groups in edit mode, though testing is looking good. [String changes made]: None
Flags: needinfo?(chrislord.net)
Attachment #8571891 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Attachment #8571891 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 37•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/435b8de3cce7316cbfc326603a4a554fb000a366
Comment 38•9 years ago
|
||
This bug has been successfully verified on latest Flame v2.2&3.0. On Edit mode,short "long press" looks work well and responsive. See attachment: verified_v2.2.mp4. Reproduce rate: 0/5 STR: 1.Long press any icon on Homescreen. 2.Long tap any icons or groups,and drag them,then quickly long tap other app icon. **Short "long press" looks work well and responsive. Flame 2.2 (Pass): Build ID 20150319002500 Gaia Revision 9043c11f699c15bb6072422d1dad6518d1b5ddda Gaia Date 2015-03-19 01:40:44 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c0442d170bec Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150319.042028 Firmware Date Thu Mar 19 04:20:38 EDT 2015 Bootloader L1TC000118D0 Flame 3.0 (Pass): Build ID 20150319160212 Gaia Revision c39e15f631de80c69467fda0d4ea0bcda9e194ca Gaia Date 2015-03-18 19:30:04 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/cbd0efcd976c Gecko Version 39.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150319.193329 Firmware Date Thu Mar 19 19:33:42 EDT 2015 Bootloader L1TC000118D0
Status: RESOLVED → VERIFIED
Keywords: verifyme
Comment 39•9 years ago
|
||
Updated•9 years ago
|
QA Whiteboard: [MGSEI-Triage+]
You need to log in
before you can comment on or make changes to this bug.
Description
•