Closed Bug 1385934 Opened 7 years ago Closed 7 years ago

[RTL] Topsites in Activity Stream is right-aligned: should be center-aligned

Categories

(Firefox for Android Graveyard :: General, defect, P1)

defect

Tracking

(firefox57 fixed)

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

(Whiteboard: [mobileAS])

Attachments

(4 files)

Whiteboard: [mobileAS]
Assignee: nobody → michael.l.comella
Iteration: --- → 1.28
Priority: P2 → P1
Attached image Screenshot on launch
The top sites panel looks fine on launch. The highlights introduction is incorrect and causes the page to scroll quite far - bug 1337425.
Top sites looks fine full and correctly scrolls for more options from right to left. However, the highlights items is not correct: - the page domain text should be right-aligned - "Visited" should be right-aligned to the icon.
(In reply to Michael Comella (:mcomella) from comment #2) > However, the highlights items is not correct: Filed bug 1393274 so we can keep this bug and verify that we verified the top sites panel RTL already.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
There's another issue here- the Top Sites tiles at the top are aligned to the right, but should be aligned to the center.
(In reply to ItielMaN from comment #4) > There's another issue here- the Top Sites tiles at the top are aligned to > the right, but should be aligned to the center. Thanks for pointing that out! That is weird...
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
It looks like we're using a third-party RtlViewPager [1] for those items. We imported it in 2017 [2], which is after the latest commit to the repository (2016) [3]. There are no open issues for this bug (but it's a small project - 72 stars & 20 forks) so it's possible that this is an error in our code. [1]: https://github.com/diego-gomez-olvera/RtlViewPager [2]: https://hg.mozilla.org/mozilla-central/log/tip/mobile/android/thirdparty/com/booking/rtlviewpager/RtlViewPager.java [3]: https://github.com/diego-gomez-olvera/RtlViewPager/commits/master
Summary: [RTL] Verify Topsites in Activity Stream works in RTL → [RTL] Topsites in Activity Stream is right-aligned: should be center-aligned
(In reply to Michael Comella (:mcomella) from comment #6) > It looks like we're using a third-party RtlViewPager [1] for those items. We > imported it in 2017 [2], which is after the latest commit to the repository > (2016) [3]. There are no open issues for this bug (but it's a small project > - 72 stars & 20 forks) so it's possible that this is an error in our code. FWIW, I remember it worked okay on previous implementation of the Top Sites tiles.
Iteration: 1.28 → 1.29
It looks like the problem is we specify `paddingLeft` in activity_stream_topsites_page.xml [1] which goes wrong when we switch to RTL. fwiw, the paddingEnd looks like it exists because of the margins between the views in the ViewPager (rather than an explicit margin that maybe have been specified). [1]: http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/mobile/android/app/src/main/res/layout/activity_stream_topsites_page.xml#6
Comment on attachment 8902483 [details] Bug 1385934: Add license to activity_stream_topsites_page.xml. https://reviewboard.mozilla.org/r/174066/#review179320
Attachment #8902483 - Flags: review?(liuche) → review+
Comment on attachment 8902484 [details] Bug 1385934: Use RTL layout attr in activity_stream_topsites_page. https://reviewboard.mozilla.org/r/174068/#review179322 lgtm! ::: mobile/android/app/src/main/res/layout/activity_stream_topsites_page.xml:6 (Diff revision 1) > <?xml version="1.0" encoding="utf-8"?> > <!-- This Source Code Form is subject to the terms of the Mozilla Public > - License, v. 2.0. If a copy of the MPL was not distributed with this > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > + > +<!-- The paddingEnd visible to the user comes from the padding between items. As such, paddingEnd here should be 0dp. Helpful comment :)
Attachment #8902484 - Flags: review?(liuche) → review+
Pushed by michael.l.comella@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9bfb38ac67b2 Add license to activity_stream_topsites_page.xml. r=liuche https://hg.mozilla.org/integration/autoland/rev/72ffe7066fc0 Use RTL layout attr in activity_stream_topsites_page. r=liuche
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Confirmed fixed on latest Nightly.
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: