Closed Bug 1150706 Opened 10 years ago Closed 10 years ago

[RTL] [Camera] Preview swiping is not reversed

Categories

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

x86
macOS
defect

Tracking

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

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

People

(Reporter: tif, Assigned: dmarcos)

References

Details

Attachments

(4 files)

Attached file RTL preview.pdf
STR: 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 Actual: Additional photos are found to the right and are reached by swiping left. The same interaction as in a non-RTL language. Expected: 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? Thanks Hema
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]: None
Attachment #8591925 - Flags: feedback? → approval-gaia-v2.2?(doliver)
Attachment #8591925 - Flags: approval-gaia-v2.2?(doliver) → approval-gaia-v2.2+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 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 https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cc02ee38b252 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
Status: RESOLVED → VERIFIED
Test case has been added in moztrap: https://moztrap.mozilla.org/manage/case/15921/
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: