Right-to-left CSS (using module for conversion) #7072

mapmeld · 2019-08-27T04:27:17Z

Replacing #6822 with this smaller pull request, thanks to @Gudahtt

Simple changes such as margin-left, margin-right, etc. are covered automatically by gulp-rtlcss. There is an alternate index-rtl.css file which activates on RTL languages
More complex CSS which should not be overwritten (direction: ltr on Identicon and 0x addresses) protected with an /*rtl:ignore*/ comment
Other settings (dir="auto" on input, rotating arrow images) are set directly

The language direction was stored in state twice -`textDirection` was used in the background, and `languageDirection` was used in the UI. `textDirection` is now used in both places instead.

A second stylesheet has been added to each HTML page for use with right-to-left locales. It is disabled by default. It is enabled on startup if a RTL locale is set, and when switching to a RTL locale. Similarly the LTR stylesheet is disabled when a RTL locale is used. Unfortunately there is an unpleasant flash of unstyled content when switching between a LTR and a RTL locale. There is also a slightly longer page load time when using a RTL locale (<1s difference). We couldn't think of an easy way to avoid these problems.

Gudahtt

This looks great, thanks!

Gudahtt

My earlier review was mainly from reviewing the code; I hadn't looked over the design in great detail at the time. After further review I have found a few issues that should probably be addressed if they're not too challenging

I found a few more icons that seem like they should be flipped around:

ui/app/pages/send/send.scss - The caret under .send__select-recipient-wrapper__list__back-caret
This is the caret next to the "Back to All" button in the "Send" flow (e.g. after clicking "Transfer between my accounts")
ui/app/pages/settings/contact-list-tab/index.scss - under .address-book__my-accounts-button__caret
This is in the 'Contacts' settings page, next to "My Wallet Accounts"
ui/app/pages/settings/networks-tab/index.scss - under .networks-tab__back-button
This is on the 'Networks' settings, possibly only in the browser-action popup UI (not fullscreen)

The toggle buttons in the settings seem pretty broken (e.g. on the 'Advanced' tab):

I don't think it'd be easy to make the toggle support RTL properly (we're using a library that isn't actively maintained). So maybe we should just leave the toggle as LTR for now, and replace that library at some point in the future. Or you could try to do something clever like reverse the styles and the values when using RTL.

I found a few input fields that should probably be set to left-to-right, as they're used for numbers or addresses:

ui/app/pages/send/send.scss - the class ens-input__wrapper__input
This is from the first page of the 'Send' flow, when choosing the recipient.
ui/app/components/app/gas-customization/gas-modal-page-container/advanced-tab-content/index.scss - the class advanced-tab__gas-edit-row__input
This is on the screen for editing the gas value during the 'Send' flow
ui/app/components/aoo/gas-customization/advanced-gas-inputs/index.scss - the class .advanced-gas-inputs__gas-edit-row__input
This is similar to the last one, except it's in the edit gas page of the side-bar that appears in some cases (e.g. when speeding up a transaction)
The id search-token. There are no classes for this, but you could add it to ui/app/pages/add-token/index.scss, or create a new file in ui/app/pages/add-token/token-search perhaps.
This is the field for adding new tokens. I'm a bit on the fence about this one - maybe it's fine as-is? But all of the tokens are left-to-right, so it seems like the search field should be as well perhaps.
Most of the fields in the Custom RPC form should probably be left-to-right? The first one is just a name, so that's fine as right-to-left, but the others are numbers, URLs, etc. I'm not sure what to do with these - I'll leave them to your discretion.
The styles for this are in ui/app/pages/settings/networks-tab
The 'Add site' field in the 'Connections' tab of the settings.
This one just uses a shared classname at the moment - I guess a new class could be added to the text field specifically, which could be put in /ui/app/pages/settings/connections-tab/index.scss.

Phew - that's a lot of stuff! That's all I found at the moment. Please feel free to push back on any of these if you think the suggestion is wrong or misguided or not important enough to block this getting merged - I'm not a right-to-left user, so this is all pretty new to me.

mapmeld · 2019-09-03T04:55:14Z

Thanks for digging deep into the UI and finding these! I essentially agree with all of the changes, with these additional notes

I flipped the Contacts > My Wallet Accounts icon, but in the build it looks like it is being written as a text string '>' (which correctly appears as '<' in RTL text).
Toggle button is indeed tricky, and is easier to understand for now with direction: ltr, it's in General Settings and in Security & Privacy
ENS input is a new feature since I started the previous PR - I'm making text input dir="auto" in the HTML, so typing LTR or RTL text will start from the left or right automatically. Typing 0x... addresses is LTR. This leaves the door open if someone had RTL in contacts or in ENS.
Same as above for token search
Custom RPC inputs ought to be all LTR content, so I did set the form inputs as LTR

For extra reading (or future maintainers reading the thread) I have a write-up on some RTL settings here https://mapmeld.com/rtl-guide/

Gudahtt · 2019-09-03T16:01:55Z

I flipped the Contacts > My Wallet Accounts icon, but in the build it looks like it is being written as a text string '>' (which correctly appears as '<' in RTL text).

I believe the caret written as a text string is in the breadcrumb component - as opposed to the one referenced in the stylesheet, which is alongside the "My Wallet Accounts" entry. It looks correct now in any case.

Toggle button is indeed tricky, and is easier to understand for now with direction: ltr, it's in General Settings and in Security & Privacy

Looks much better! Though it would be nice if it was right-aligned.

Custom RPC inputs ought to be all LTR content, so I did set the form inputs as LTR

I'm not entirely sure this was the best choice for the first field, since as a user-chosen name it could be either LTR or RTL. Leaving it as LTR seems fine for the time being though - we can always change it later.

I'm making text input dir="auto" in the HTML, so typing LTR or RTL text will start from the left or right automatically.

Fantastic - using dir="auto" does seem like the right choice for all of these cases. I noticed that it didn't appear to be working for the TextField components though - it needs to be passed down to the underlying input component. I've managed to do that locally, so I'll push this change up. I'll also update the toggle to be right-aligned.
(Edit: Nevermind, it seems that I can't push to this branch. I've pushed to mine instead - these two changes are here and here)

I noticed one more set of fields that should be made dir="auto" - the "add custom token" fields. I was wondering though - should we just set dir="auto" for the base TextField component? It does seem like a better default for input components than not specifying a direction.

mapmeld · 2019-09-03T16:42:21Z

I think you're right to make TextField dir='auto' by default (we can make it possible to overwrite, but I can't think of a reason where you it's needed)

This might also remove the need to put direction: ltr on the network-form?

Gudahtt · 2019-09-03T16:43:28Z

This might also remove the need to put direction: ltr on the network-form?

Yes, I think so

mapmeld · 2019-09-03T16:47:29Z

OK I made your suggested changes but actually I don't know how to set the default attribute on TextField

Gudahtt · 2019-09-03T16:50:52Z

I believe this should do it: 57a6b0d

This also allows you to remove any line where dir="auto" was set explicitly on a TextField.

mapmeld · 2019-09-03T16:58:20Z

Alright, that default is added in. I've removed the rule from network-form and a few other dir="auto"s which I set on TextFields across the app

Gudahtt

Looks great, thanks! That covers everything for me.

mapmeld and others added 7 commits August 21, 2019 18:11

gulp-rtl approach

10e154c

dont forget gulp-rename

084a7b4

Update lockfile

968468e

Add missing import

f6d454e

Remove languageDirection state

581be59

The language direction was stored in state twice -`textDirection` was used in the background, and `languageDirection` was used in the UI. `textDirection` is now used in both places instead.

patch rtl:ignore

b01fef5

mapmeld requested review from danjm, Gudahtt and whymarrh as code owners August 27, 2019 04:27

restoring previously requested change of indexOf -> includes

a1b2282

Gudahtt approved these changes Aug 29, 2019

View reviewed changes

Gudahtt requested changes Aug 31, 2019

View reviewed changes

mapmeld force-pushed the rtlcss_module branch from f22ad30 to cc0d5e5 Compare September 3, 2019 03:40

recommended RTL changes

047dbf4

mapmeld force-pushed the rtlcss_module branch from cc0d5e5 to 047dbf4 Compare September 3, 2019 04:22

apply @Gudahtt changes to toggle and TextField

68ae99d

mapmeld added 2 commits September 3, 2019 16:52

dir=auto default, fixes network tab inputs

8c4593d

remove extra dir=auto on TextFields

6ecc4a2

Gudahtt approved these changes Sep 3, 2019

View reviewed changes

Gudahtt merged commit d589d2d into MetaMask:develop Sep 3, 2019

bbondy mentioned this pull request Dec 16, 2019

Rebase Crypto Wallets to MetaMask upstream brave/brave-browser#7408

Closed

brad-decker mentioned this pull request Jul 27, 2020

Make UI support right-to-left languages #5436

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Right-to-left CSS (using module for conversion) #7072

Right-to-left CSS (using module for conversion) #7072

mapmeld commented Aug 27, 2019 •

edited

Loading

Gudahtt left a comment

Gudahtt left a comment •

edited

Loading

mapmeld commented Sep 3, 2019 •

edited

Loading

Gudahtt commented Sep 3, 2019 •

edited

Loading

mapmeld commented Sep 3, 2019

Gudahtt commented Sep 3, 2019

mapmeld commented Sep 3, 2019

Gudahtt commented Sep 3, 2019

mapmeld commented Sep 3, 2019

Gudahtt left a comment

Right-to-left CSS (using module for conversion) #7072

Right-to-left CSS (using module for conversion) #7072

Conversation

mapmeld commented Aug 27, 2019 • edited Loading

Gudahtt left a comment

Choose a reason for hiding this comment

Gudahtt left a comment • edited Loading

Choose a reason for hiding this comment

mapmeld commented Sep 3, 2019 • edited Loading

Gudahtt commented Sep 3, 2019 • edited Loading

mapmeld commented Sep 3, 2019

Gudahtt commented Sep 3, 2019

mapmeld commented Sep 3, 2019

Gudahtt commented Sep 3, 2019

mapmeld commented Sep 3, 2019

Gudahtt left a comment

Choose a reason for hiding this comment

mapmeld commented Aug 27, 2019 •

edited

Loading

Gudahtt left a comment •

edited

Loading

mapmeld commented Sep 3, 2019 •

edited

Loading

Gudahtt commented Sep 3, 2019 •

edited

Loading