Closed Bug 1337933 Opened 3 years ago Closed 3 years ago
[RTL] Tabs grid layout item decoration paddings are reversed in RTL
The item decorations on tabs in the grid view have different left and right paddings, dependent on their position in a given row. It looks like those paddings are reversed in RTL mode. This affects tablets, phones in landscape mode, and phones in portrait mode using the "compact tabs" setting. I think we need to keep the tabs layout RecyclerView in RTL mode in order to get the tabs to layout in the correct order, but the padding on those tabs shouldn't have its sense of left and right switched since it really is checking something like "are we the leftmost tab in a row? then don't put any padding to our left" (it's a little more complicated than that in general). If there's no way to turn off RTL just for those decorations then I guess we need to add a check for RTL in the ItemDecoration itself? (By the way I'm testing by switching the Android language setting to an RTL language - please let me know if that's not a sufficiently correct way to do it.)  https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/widget/GridSpacingDecoration.java#43-46
Dupe of 1337897?
See Also: → 1337897
(In reply to Tomer Cohen :tomer from comment #1) > Dupe of 1337897? Not fully duplicate, as the other bug is only about having Compact Tabs enabled in Portrait Mode, while this issue remove the the dependency on Compact Tabs on landscape mode, and having it on portrait mode while Compact Tabs is enabled.
(In reply to Tomer Cohen :tomer from comment #1) > Dupe of 1337897? I think they'll actually be fixed in different places in different ways, so I think they're separate issues. This bug is (basically) about the distance between the thumbnails (I should have made that clearer), which is a runtime-computed value (computed at  in comment 0). Bug 1337897 is about the distance between the close button and the tab title, which is set in xml layout, if I'm not mistaken.
I had what was RTL and not RTL reversed in my head in the first version of the patch, which gave the same result, but hopefully the second version makes more sense :)
Comment on attachment 8855581 [details] Bug 1337933 - Make the tabs grid layout decorations work with RTL. https://reviewboard.mozilla.org/r/127424/#review130206 LGTM
Attachment #8855581 - Flags: review?(max) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/847803cc7003 Make the tabs grid layout decorations work with RTL. r=maliu
Confirmed fixed in latest Nightly on Samsung Galaxy S5. If someone can verify this also on a tablet, I'd suggest uplifting it to beta.
Comment on attachment 8855581 [details] Bug 1337933 - Make the tabs grid layout decorations work with RTL. Approval Request Comment [Feature/Bug causing the regression]: RTL on android [User impact if declined]: Spacing between tabs in the tabs trays that show tabs in a grid will be off (see the attachment for an example) - tablets, phones in landscape mode, and phones in portrait mode with "Compact tabs" on are affected. [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: Take a screenshot of the tabs tray on a phone with landscape orientation in LTR and RTL modes and compare the spacing between the tabs: it should be the same in LTR and RTL. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: Low risk. [Why is the change risky/not risky?]: For RTL mode we're just reusing the spacing calculations done in LTR and taking into account the way position indices change when in RTL mode. [String changes made/needed]: None.
Attachment #8855581 - Flags: approval-mozilla-aurora?
Comment on attachment 8855581 [details] Bug 1337933 - Make the tabs grid layout decorations work with RTL. Fix an RTL issue and was verified. Aurora54+.
Attachment #8855581 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.