Closed Bug 945776 Opened 11 years ago Closed 10 years ago

[Haida] Edge Gestures conflict with cropping tool handles

Categories

(Firefox OS Graveyard :: Gaia::Gallery, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v1.4 affected, b2g-v2.0 fixed)

RESOLVED FIXED
2.0 S3 (6june)
blocking-b2g 2.0+
Tracking Status
b2g-v1.4 --- affected
b2g-v2.0 --- fixed

People

(Reporter: dmarcos, Assigned: pdahiya)

References

Details

(Keywords: regression, Whiteboard: permafail)

Attachments

(2 files)

Steps to reproduce on an engineering build:

1. Go to camera
2. Take a picture with the device in landscape orientation
3. Go to Settings-> Device Information -> Developer
4. Check 'Edge gesture enabled'
5. Go to gallery
6. Edit the previously taken picture in landscape orientation
7. It is very difficult to interact with the crop handles on left/right sides of the screen. The event is captured by the edge gesture handler.
Hey UX team!
This one is tricky, since the gesture you do to drag the crop handle horizontally is exactly the same as the one you do to swipe between apps.

Any idea?


Note that if you long press (for ~0.5sec) before you start dragging horizontally it should move the handle correctly.
Flags: needinfo?(firefoxos-ux-bugzilla)
Assigning to Rob. He'll be at the work week so you guys can discuss then too.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(rmacdonald)
(In reply to Etienne Segonzac (:etienne) from comment #1)
> Hey UX team!
> This one is tricky, since the gesture you do to drag the crop handle
> horizontally is exactly the same as the one you do to swipe between apps.
> 
> Any idea?
> 
> 
> Note that if you long press (for ~0.5sec) before you start dragging
> horizontally it should move the handle correctly.

Hi everyone... 

I'm still having challenges with it in spite of Etienne's efforts and this may be a case where we need to adjust the UI to move the photo away from the edge of the screen. Flagging Peter for his thoughts on visual design changes.

- Rob
Flags: needinfo?(rmacdonald) → needinfo?(pla)
Whiteboard: burirun1.3-3
Flagging Gordon in addition to Peter, so we can get some eyes on this ASAP.
Flags: needinfo?(gbrander)
From a UX perspective, it's totally acceptable for the crop mode to overrule edge gestures.

Edge gestures should be consistent, and part of this consistent behavior is that they are overruled by full-screen modal contexts like alerts or, in this case, the crop tool.

Another way to think about it is to imagine what the user would expect in a given scenario. Since the user activated the crop tool most recently and the drag handles are present, chances are that's what's on their mind when they drag it.

From a technical standpoint, this may be a bit tricky as-implemented, since edge gestures are currently "first me, then you". A couple of off-the-cuff ideas:
* Perhaps app could put its frame into a "modal" mode, bumping it up the z-index stack.
* Or instead of using a bumper element, edge gesture could be derived from any uncaught move event that has a start event with coords in the correct screen area. E.g. "first you, then me".
Flags: needinfo?(gbrander)
Came across this while doing test case: https://moztrap.mozilla.org/manage/case/2307/

It is difficult to use pinch gestures to zoom-in and zoom-out as well. Might this be the same issue? If not I can create a separate bug.

See: http://youtu.be/66oCCqDNa00

1.3 Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140210004002
Gaia: 5c8416fb1ea4a27f172ee6386ab3c19135448506
Gecko: 9c9382f433c0
Version: 28.0
Firmware Version: v1.2-device.cfg
(In reply to Lionel Mauritson from comment #6)
> Came across this while doing test case:
> https://moztrap.mozilla.org/manage/case/2307/
> 
> It is difficult to use pinch gestures to zoom-in and zoom-out as well. Might
> this be the same issue? If not I can create a separate bug.

This is tracked on bug 946809, we know how to fix it :)
(In reply to Gordon Brander :gbrander from comment #5)
> From a UX perspective, it's totally acceptable for the crop mode to overrule
> edge gestures.
> 
> Edge gestures should be consistent, and part of this consistent behavior is
> that they are overruled by full-screen modal contexts like alerts or, in
> this case, the crop tool.

The system app, where the edge gestures code lives has *no* idea of what the user is doing inside the gallery app.

So we might need a fallback plan since we probably won't have a way for an app to declare itself in modal mode.
Hey guys,

I agree with Gordon.  I think the idea of possibly shrinking the photo *could* work but feels counterintuitive to me - "you want to edit this photo?  ok, we're gonna make it harder but shrinking it".

I like the solution of the modal edit overriding edge gesture (Rob, you might want to provide a sanity check here as I'm not sure how much of an impact that will have on sheets).

Etienne, you mentioned the system code that handles gestures doesn't know about modes - but maybe it should?  Is this something that is a possibility that we can add?  

I'd prefer not to do a short-term fix here, unless it's a good one.  Will keep thinking about it... and post back if I think of anything.

Thoughts?
Flags: needinfo?(rmacdonald)
Flags: needinfo?(pla)
Flags: needinfo?(gbrander)
Flags: needinfo?(etienne)
(In reply to Peter La from comment #9)
> I like the solution of the modal edit overriding edge gesture (Rob, you
> might want to provide a sanity check here as I'm not sure how much of an
> impact that will have on sheets).
> 

I like it too, but...

> Etienne, you mentioned the system code that handles gestures doesn't know
> about modes - but maybe it should?

I meant we don't have a semantic way and an API available for an webapp to express "I'm doing a modal operation right now but it's not a prompt nor an alert" (that I know of).
It's not really a "maybe it should" matter :/
Flags: needinfo?(etienne)
Diego, was the crop image resized in more recent versions? I'm trying it and it seems to be further away from the edges.
Flags: needinfo?(rmacdonald)
Flags: needinfo?(gbrander)
Diego - see comment 11. Forgot to NI you on my question. :)
Flags: needinfo?(dmarcos)
It was changed but not recently. This the latest tweak that we've done to the crop tool (three months ago).

https://bugzilla.mozilla.org/show_bug.cgi?id=935072

This is a challenging UX issue. Isn't it? :) Looking forward to seeing what you come up with.
Flags: needinfo?(dmarcos)
Whiteboard: burirun1.3-3 → burirun1.3-3, burirun1.4-1
Whiteboard: burirun1.3-3, burirun1.4-1 → permafail
Blocks: 991849
No longer blocks: edge-gestures
Flags: needinfo?(tshakespeare)
Hey Rob - I was talking with Diego about this just now. Do you know when edge gestures are going to be enabled by default? Starting with that info we can see how much time we have for the fix.

Thanks!
Flags: needinfo?(rmacdonald)
Looking at Etienne's email today, it looks like edge gestures are going to be turned on by default in 2.0. I think we'll need a quick/easy solution to ensure this isn't broken and then we can take a look at something more elegant when we do the gallery redesign.

Diego - can we make the image smaller so that the two gestures don't conflict?
Flags: needinfo?(tshakespeare)
Flags: needinfo?(rmacdonald)
Flags: needinfo?(dmarcos)
feature-b2g: --- → 2.0
Can we get a video describing the bug here with the latest trunk?
Keywords: qawanted
Blocks: edge-gestures
No longer blocks: 991849
QA Contact: ddixon

(In reply to Jason Smith [:jsmith] from comment #16)
> Can we get a video describing the bug here with the latest trunk?


Link to video: https://www.youtube.com/watch?edit=vd&v=bt0D9kyh-R4


Open_C  2.0 trunk variables

Environmental Variables:
Device: Open_C 
BuildID: 20140516073017
Gaia: 799b0f8bb71bc1b944f90c117ab5d6be4837ba1f
Gecko: b5b35ec88db8
Version: 32.0a1
Firmware Version: P821A10V1.0.0B06_LOG_DL
Keywords: qawanted
Technically this would be a regression because this is preffed on and impacts functionality that used to work correctly without edge gestures. The video gives the impression that the UX impact is pretty bad, as the crop handles are very hard to use in a horizontal workflow.
blocking-b2g: --- → 2.0?
feature-b2g: 2.0 → ---
Keywords: regression
Assignee: nobody → pdahiya
Summary: Edge Gestures conflict with cropping tool handles → [Haida] Edge Gestures conflict with cropping tool handles
This issue is a side effect of edge gestures feature that is being enabled in 2.0... haida/edge gestures feature is not a blocker for 2.0, however if we are turning this on, we need to fix its impact on gallery and FMRadio -- I am moving this into sprint 3 milestone as haida support work.
Target Milestone: --- → 2.0 S3 (6june)
blocking-b2g: 2.0? → 2.0+
(In reply to Tiffanie Shakespeare from comment #15)
> Looking at Etienne's email today, it looks like edge gestures are going to
> be turned on by default in 2.0. I think we'll need a quick/easy solution to
> ensure this isn't broken and then we can take a look at something more
> elegant when we do the gallery redesign.
> 
> Diego - can we make the image smaller so that the two gestures don't
> conflict?

Hi Tif,
We can reduce the preview-canvas area. I have tried to fix edge gesture conflict by adding 1 rem space on both sides and reducing width of preview-canvas by 2 rem. Attaching the screen shot of original and smaller preview canvas area. 
Please confirm if this works with UX. Thanks
Flags: needinfo?(dmarcos) → needinfo?(tshakespeare)
Hey Punam - that looks like it could work. Do you have a patch I could try out on my device? Thanks!

(In reply to Punam Dahiya from comment #22)
> Created attachment 8428088 [details]
> Screen shots of reduced preview canvas area while editing an image
Flags: needinfo?(tshakespeare) → needinfo?(pdahiya)
Here's the link to patch,

https://github.com/mozilla-b2g/gaia/pull/19612

Thanks
Flags: needinfo?(pdahiya)
Seems good to me - I could still activate the edge gestures but it wasn't often and cropping landscape photos is now actually doable. 

Rob/Amy - do you have any feedback?
Hi Tif

I have updated the patch by bringing the left and right sides only in by 1 rem. This gives us the benefit of leaving the height as is and only reducing the width by 2 rem. 
(Patch in #comment 24 it was reducing the preview canvas area by 1 rem from all sides).

https://github.com/mozilla-b2g/gaia/pull/19612

Please provide your feedback. 

Also, would like to get your thoughts on changing the crop handles visually, so that they appear like semi-circles on the inside of the frame. That would encourage the user to touch inside the frame rather than on the edge.  No assets are required for this change as the draw is done with a canvas.

Thanks
Flags: needinfo?(tshakespeare)
Hey Punam! 

Hurray! We can actually crop landscape photos!

After trying it out a few times on the device, I think you're right about the cropping handles, if they were to appear more inside the frame I think that would further encourage the user to tap inside which will reduce the odds of accidentally activating the edge gesture.

NI'ing Amy for some visual design direction.

Also NI'ing Rob to take a look.

Thanks!
Flags: needinfo?(tshakespeare)
Flags: needinfo?(rmacdonald)
Flags: needinfo?(amlee)
Hi Tif, Amy

Here' another attempt to further refine edge gesture conflict with crop handles by increasing crop handle sizes to 40px from 25px. IMO, the experience has improved much better.

PR with crop handle size 40 px and width reduced 1 rem from each side
https://github.com/mozilla-b2g/gaia/pull/19784

PR with width reduced 1 rem from each side
https://github.com/mozilla-b2g/gaia/pull/19612

Please give your feedback. Thanks
Flags: needinfo?(tshakespeare)
(In reply to Tiffanie Shakespeare from comment #27)
> Hey Punam! 
> 
> Hurray! We can actually crop landscape photos!
> 
> After trying it out a few times on the device, I think you're right about
> the cropping handles, if they were to appear more inside the frame I think
> that would further encourage the user to tap inside which will reduce the
> odds of accidentally activating the edge gesture.
> 
> NI'ing Amy for some visual design direction.
> 
> Also NI'ing Rob to take a look.
> 
> Thanks!

I'm good with the smaller circles when dragging out the crop edges.
Flags: needinfo?(amlee)
Seems good then - let's just stick with the small circles then. Everything else looks good. Thanks for all your work Punam!
Flags: needinfo?(tshakespeare)
Thanks Tif, Amy. 

David, Attached patch fixes edge gesture conflict by reducing width by 1 rem from both sides and keeping height as is. Please review. Thanks
Attachment #8431185 - Flags: review?(dflanagan)
(In reply to Amy Lee [:amylee] from comment #29)
> 
> I'm good with the smaller circles when dragging out the crop edges.

Amy,

I think that what Tif set needinfo for was the idea Punam expressed in comment #26: if we change the look of the crop handles so that they are inside the frame rather than outside the frame (maybe semi-circles inside the frame that are always visible, not just when the user touches) then we invite the user to touch inside the frame and pull it smaller rather than touching outside the frame and push it smaller...

Punam's alternative patch in comment 28 was a separate experiment. Thanks for your feedback on that, but I'd still be interested to hear what you think of actually changing how the crop handles are drawn.
Flags: needinfo?(amlee)
Harvey,

You set the affected flag on this bug for v1.4.  But the bug is about edge gestures which are not enabled in 1.4.  Could you explain please?  I know there are other bugs that make it difficult to work with the crop handles, and we might want to get those fixed in 1.4, but this bug probably isn't the place to do it!

Given that the patch here is explicitly about edge gestures we probably can't uplift this to 1.4. So if you need something fixed in that release, please find or file a different crop handle bug and bring it to our attention.
Flags: needinfo?(jharvey)
(In reply to David Flanagan [:djf] from comment #33)
> Harvey,
> 
> You set the affected flag on this bug for v1.4.  But the bug is about edge
> gestures which are not enabled in 1.4.  Could you explain please?  I know
> there are other bugs that make it difficult to work with the crop handles,
> and we might want to get those fixed in 1.4, but this bug probably isn't the
> place to do it!
> 
> Given that the patch here is explicitly about edge gestures we probably
> can't uplift this to 1.4. So if you need something fixed in that release,
> please find or file a different crop handle bug and bring it to our
> attention.

I think this was marked 1.4 affected because he was able to reproduce the bug when edge gestures was preffed on. Agree though we do not need to take this to 1.4, as edge gestures is preffed off by default on 1.4.
Flags: needinfo?(jharvey)
Comment on attachment 8431185 [details] [review]
Patch with fix for Bug945776

Punam,

This patch looks good to me.  It fixes the edge gesture issue and is good enough for 2.0. Perhaps we can reconsider the crop handle design for a later release.

We are still left with the problem that it is very hard to activate the crop handles even when they are not near the left or right edge. That is a separate issues (and I suspect the reason that this bug is marked as affecting v1.4) but it is one we ought to look into.  I assume it is just a matter of changing the way hit detection (to determine if we have started to drag a handle) is done.
Attachment #8431185 - Flags: review?(dflanagan) → review+
Flags: needinfo?(rmacdonald)
I think I've figured out the other bug that makes it hard to grab the handles and will find or file a different bug to attach a patch to.  We should just land this one to fix the edge gesture conflict on master, and then we can land and uplift the other one to fix all the affected releases.
Let's use bug 1020273 for the other bug.
(In reply to David Flanagan [:djf] from comment #32)
> (In reply to Amy Lee [:amylee] from comment #29)
> > 
> > I'm good with the smaller circles when dragging out the crop edges.
> 
> Amy,
> 
> I think that what Tif set needinfo for was the idea Punam expressed in
> comment #26: if we change the look of the crop handles so that they are
> inside the frame rather than outside the frame (maybe semi-circles inside
> the frame that are always visible, not just when the user touches) then we
> invite the user to touch inside the frame and pull it smaller rather than
> touching outside the frame and push it smaller...
> 
> Punam's alternative patch in comment 28 was a separate experiment. Thanks
> for your feedback on that, but I'd still be interested to hear what you
> think of actually changing how the crop handles are drawn.

I like the idea of the semi-circles inside the crop frame. I think having them inside the frame will direct the user to have their finger inside the frame rather than right at the edge. Tiff thoughts? Would it be possible to see what this looks like?
Flags: needinfo?(amlee) → needinfo?(tshakespeare)
(In reply to David Flanagan [:djf] from comment #35)
> Comment on attachment 8431185 [details] [review]
> Patch with fix for Bug945776
> 
> Punam,
> 
> This patch looks good to me.  It fixes the edge gesture issue and is good
> enough for 2.0. Perhaps we can reconsider the crop handle design for a later
> release.
Thanks David for review, patch landed on master
https://github.com/mozilla-b2g/gaia/commit/c0a35681a24bdaf11ef643501e62b2be4b5f6937

I will open a new bug for tracking crop handle visual design.
> 
> We are still left with the problem that it is very hard to activate the crop
> handles even when they are not near the left or right edge. That is a
> separate issues (and I suspect the reason that this bug is marked as
> affecting v1.4) but it is one we ought to look into.  I assume it is just a
> matter of changing the way hit detection (to determine if we have started to
> drag a handle) is done.

Agreed, we can use bug 1020273 for fixing this issue.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to Amy Lee [:amylee] from comment #38)
> (In reply to David Flanagan [:djf] from comment #32)
> > (In reply to Amy Lee [:amylee] from comment #29)
> > > 
> > > I'm good with the smaller circles when dragging out the crop edges.
> > 
> > Amy,
> > 
> > I think that what Tif set needinfo for was the idea Punam expressed in
> > comment #26: if we change the look of the crop handles so that they are
> > inside the frame rather than outside the frame (maybe semi-circles inside
> > the frame that are always visible, not just when the user touches) then we
> > invite the user to touch inside the frame and pull it smaller rather than
> > touching outside the frame and push it smaller...
> > 
> > Punam's alternative patch in comment 28 was a separate experiment. Thanks
> > for your feedback on that, but I'd still be interested to hear what you
> > think of actually changing how the crop handles are drawn.
> 
> I like the idea of the semi-circles inside the crop frame. I think having
> them inside the frame will direct the user to have their finger inside the
> frame rather than right at the edge. Tiff thoughts? Would it be possible to
> see what this looks like?

Created bug 1021067 for tracking visual updates to crop handle. Tif, please use bug 1021067 to provide requested info. Thanks
Clearing NI...
Flags: needinfo?(tshakespeare)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: