In Edit Mode, long press (to initiate move group or move app icon) is too long to trigger action, causes user confusion

VERIFIED FIXED in 2.2 S7 (6mar)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: kcaldwell, Assigned: cwiiis)

Tracking

({polish})

unspecified
2.2 S7 (6mar)
All
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [systemsfe], [ux-most-wanted])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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
(Reporter)

Comment 1

4 years ago
please also see Bug 1092325 - Remove need for long press when moving icons/groups in edit mode (edit)
(Assignee)

Comment 2

4 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
Whiteboard: [systemsfe]
(Assignee)

Comment 3

4 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)
(Reporter)

Comment 4

4 years ago
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

4 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

4 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

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 7

4 years ago
Updated branch to latest master, which contains all current app-grouping fixes.
(Assignee)

Comment 8

4 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 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 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

4 years ago
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → All
(Reporter)

Comment 11

4 years ago
Hey Chris, this shortened 'long-press' patch feels much more responsive in edit mode.
Flags: needinfo?(kcaldwell)
(Reporter)

Updated

4 years ago
Whiteboard: [systemsfe] → [systemsfe], [ux-most-wanted]
What needs to happen for this to land?
Flags: needinfo?(chrislord.net)
(Assignee)

Comment 13

4 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)

Updated

4 years ago
See Also: → 1133383
(Assignee)

Comment 14

4 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 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 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

4 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)
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 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

4 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 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

4 years ago
nit addressed. This is definitely a patch we want to let bake for a few days.
Keywords: checkin-needed
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1133383
(Assignee)

Comment 24

4 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?
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
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 → ---
Depends on: 1138575
(Assignee)

Comment 27

4 years ago
(looking at this now, ftr)
Flags: needinfo?(chrislord.net)
(Assignee)

Comment 28

4 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

4 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.
(Assignee)

Comment 31

4 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)
blocking-b2g: 2.2? → 2.2+
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+
In master: https://github.com/mozilla-b2g/gaia/commit/ca977c05017e118a2a2ebefe2bd86a69cf968324
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
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
Please request Gaia v2.2 approval on this patch when you get a chance.
Flags: needinfo?(chrislord.net)
Target Milestone: --- → 2.2 S7 (6mar)
(Assignee)

Comment 36

4 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?
Attachment #8571891 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
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
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.