Closed Bug 1150706 Opened 9 years ago Closed 9 years ago

[RTL] [Camera] Preview swiping is not reversed


(Firefox OS Graveyard :: Gaia::Camera, defect, P1)



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

2.2 S10 (17apr)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- verified
b2g-master --- wontfix


(Reporter: tif, Assigned: dmarcos)




(4 files)

Attached file RTL preview.pdf
1. Switch phone to Arabic
2. Open Camera and take a few photos
3. Tap on icon to go to Preview.
4. Swipe right to go to new photo

Additional photos are found to the right and are reached by swiping left. The same interaction as in a non-RTL language.

Changing languages should flip the interaction and additional photos are found to the left. This correct behaviour is seen in Gallery.

Why should this block?
Gallery implements the correct behaviour. Camera preview and Gallery need to have matching interactions in order to avoid some massive user confusion.
I'm not sure which meta bug to attach this to - there isn't one for Camera.
blocking-b2g: --- → 2.2?
Flags: needinfo?(doliver)
(In reply to Tiffanie Shakespeare from comment #1)
> I'm not sure which meta bug to attach this to - there isn't one for Camera.

It's bug 1072002.
Blocks: camera-rtl
Flags: needinfo?(doliver)
RTL triage: P1 -- camera and gallery need to agree on directionality.
Priority: -- → P1
blocking-b2g: 2.2? → 2.2+
Diego or Punam: Who can take this?

Flags: needinfo?(pdahiya)
Flags: needinfo?(dmarcos)
Diego is taking this on, assigning to him.
Assignee: nobody → dmarcos
Flags: needinfo?(pdahiya)
Flags: needinfo?(dmarcos)
Attached file Pull Request on v2.2
Attachment #8591926 - Flags: review?(jdarcangelo)
Submitted PR for 2.2. On master I would like to use Justin's carrousel that already implements RTL orientation. It's ready to land but it got postponed
Comment on attachment 8591926 [details] [review]
Pull Request on v2.2

One minor comment about simplifying the left/right swap in PreviewGalleryController.prototype.handleSwipe. If you agree, go ahead and change it, but I'm fine with it either way.
Attachment #8591926 - Flags: review?(jdarcangelo) → review+
Pushed review changes + unit tests.
Comment on attachment 8591925 [details] [review]
[gaia] dmarcos:bug1150706 > mozilla-b2g:v2.2

Carry on r+ from Justin
Attachment #8591925 - Flags: review+
Comment on attachment 8591925 [details] [review]
[gaia] dmarcos:bug1150706 > mozilla-b2g:v2.2

Who should be approving the PR?
Flags: needinfo?(hkoka)
Attachment #8591925 - Flags: feedback?
(In reply to Diego Marcos [:dmarcos] from comment #12)
> Who should be approving the PR?

Please set the approval‑gaia‑v2.2 flag on the PR and provide the info requested in the form.
Flags: needinfo?(hkoka)
Comment on attachment 8591925 [details] [review]
[gaia] dmarcos:bug1150706 > mozilla-b2g:v2.2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Not RTL mode was implemented for the gallery preview carrousel
[User impact] if declined:
The carrousel in the gallery preview won't behave as expected in RTL mode
[Testing completed]:
Unit tests included
[Risk to taking this patch] (and alternatives if risky):
Low. The patch only implements small UI tweaks to reverse the swiping direction of the gallery preview on RTL mode.
[String changes made]:
Attachment #8591925 - Flags: feedback? → approval-gaia-v2.2?(doliver)
Attachment #8591925 - Flags: approval-gaia-v2.2?(doliver) → approval-gaia-v2.2+
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S10 (17apr)
Attached video video.mp4
Hi Tiffanie,
I have verified this issue on latest build of Flame 2.2 with the same steps in comment 0. And the behaviour is same as that in Gallery. Could you help to confirm this issue is fixed or not? Thanks!
See attachment:video.MP4

Device: Flame 2.2 
Build ID               20150420002502
Gaia Revision          c15a2b6d3a783813959c2b3bffd2a131f4270b9e
Gaia Date              2015-04-17 17:49:32
Gecko Revision
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150420.041634
Firmware Date          Mon Apr 20 04:16:45 EDT 2015
Bootloader             L1TC000118D0
Flags: needinfo?(tshakespeare)
QA Whiteboard: [MGSEI-Triage+]
Checked out the v2.2 branch and it looks good! Thanks Sue!
Flags: needinfo?(tshakespeare)
Per comment 16 and comment 17, change status to VERIFIED,FIXED
Test case has been added in moztrap:
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.