Fix: App changes positions of Ui elements when phone language RTL #7594

ahmdshrif · 2022-02-06T23:43:26Z

Details

Fixed Issues

$ #7476

Tests

Verify that no errors appear in the JS console

QA Steps

Set phone language to any RTL ( Settings -> Languages & input -> Languages ) chose Arabic for example
Open the app
Login & Browse
make sure the UI element is in its normal position
make sure the normal text also does not align to right instead of left
got to text input and use English keyboard (RTL language not supported)
make sure text input aligns to left
test on android and ios . and flow all previous steps.

Verify that no errors appear in the JS console

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

55b-d8534034caed.png)

Screen.Recording.2022-02-13.at.3.42.50.PM.mov

Android

ahmdshrif · 2022-02-07T00:20:55Z

ios have an issue with force LTR which is text does not align to left
so I add textAlign left as default props to our base text and TextInputs component.

parasharrajat · 2022-02-07T11:14:02Z

ios have an issue with force LTR which is text does not align to left
so I add textAlign left as default props to our base text and TextInputs component.

Could you please share your research on this? I would like to know more about this before accepting textAlign:left as a solution.

You have missed Mobile Web screenshots. Please add them.

marcaaron · 2022-02-07T18:24:52Z

@parasharrajat let me know when you are ready to approve the PR and I will jump in.

ahmdshrif · 2022-02-07T19:13:39Z

@parasharrajat Ok i add screen for mobile web .

i share many resources in issue any one of them work fine in android .
On ios all elment is work fine expet the text alignment .

I do some search on this issue and i find that i can go with 3 directions

1_ using js and react native libs (i share this in proposal)
Same result .

2- using react native liberary from objectC
What i use now and not work for text alignment.

3- i search on how native do this (without react )
There option for uiview to force app to work from left to right .
And we can add default attribute to text to alignment left .
I use both and not work for text alignment.

[[UIView appearance] setSemanticContentAttribute:UISemanticContentAttributeForceLeftToRight];

"Changing to right to left RTL programmatically - Stack Overflow" https://stackoverflow.com/questions/35151038/changing-to-right-to-left-rtl-programmatically

"iOS9 - Force left to right | Apple Developer Forums" https://developer.apple.com/forums/thread/9130

So i add the default text alignment to left
And this fix this issue on ios .
this just default value will not effect any thing.

parasharrajat · 2022-02-07T19:30:38Z

Did you try I18nManager.forceRTL(false) ? May be that works?

ahmdshrif · 2022-02-07T19:32:22Z

Yes sure .
This is the first option i talk about .

ahmdshrif · 2022-02-07T19:36:03Z

I find this issue on ios it looklike what we have now .
Lable text not get align correctly .

https://developer.apple.com/forums/thread/663724

parasharrajat · 2022-02-07T19:42:15Z

So you mean this does not work against the alignment issue. If yes, I will test the code now to see if alignment is correct everywhere.

ahmdshrif · 2022-02-07T19:45:57Z

Yes
It force the layout to work ltr
But not the text

parasharrajat · 2022-02-08T16:12:54Z

Testing shortly. on my list 2/5.

parasharrajat · 2022-02-08T20:29:33Z

Didn't get time to test it on all platforms today. I will do this first thing in the morning.

ahmdshrif · 2022-02-09T15:21:20Z

ok @parasharrajat

parasharrajat

Please add proper QA steps for all possible platforms. They should indicate how to switch to RTL on the device.

Also, Is it safe to switch RTL? I don't know Arabic so it could be difficult for me to navigate the device to switch back to English.

src/components/Text.js

src/components/TextInput/TextInputLabel/TextInputLabelPropTypes.js

src/components/TextInputFocusable/index.ios.js

src/components/TextInputWithFocusStyles.js

src/components/TextInputFocusable/index.ios.js

src/components/TextInputWithPrefix/index.js

src/components/TextInputWithName/textInputWithNamepropTypes.js

ahmdshrif · 2022-02-09T17:31:12Z

I can help if you need to switch back to English . but the easy way is to test on Emulator and you can wipe data if you can't switch back.

ahmdshrif · 2022-02-09T17:36:30Z

we can test this on a call if you need help with Arabic.

parasharrajat

I think you missed a couple of comments from previous review. Try to go through them again.

index.js

src/components/TextInput/TextInputLabel/TextInputLabelPropTypes.js

ahmdshrif · 2022-02-09T21:15:15Z

done all changes .

parasharrajat

The code looks good. But there are a few more places to be updated.

@ahmdshrif Could you please run a full check on the IOS app to see if everything is correctly aligned? Please record the video and attach it to the description only for IOS.

Picker is not aligned correctly. Profile page.

parasharrajat

Few more tweaks.

src/components/BaseInput/index.js

Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>

ahmdshrif · 2022-02-24T15:23:04Z

@parasharrajat ok done

parasharrajat

Testing again...

parasharrajat

@ahmdshrif It looks good. The only things are :
Update QA steps to reflect that:

Keep the Keyboard language to English. Arabic is not supported.
Test on Both Android and IOS.

ahmdshrif · 2022-02-24T19:22:40Z

@parasharrajat ok done.
thakns

parasharrajat

LGTM.

marcaaron · 2022-02-24T23:59:11Z

src/components/BaseInput/index.js

+    forwardedRef: () => {},
+};
+
+const BaseInput = props => (


Ah, sorry I hate to do this, but BaseInput is a pretty generic name and BaseTextInput is already taken... can we call this RNTextInput.js instead?

ok done. thanks .

Thanks for your patience here 🙇

thanks. 😃

OSBotify · 2022-02-25T18:02:20Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-03-01T19:54:11Z

🚀 Deployed to staging by @marcaaron in version: 1.1.41-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	failure ❌

OSBotify · 2022-03-02T10:47:29Z

🚀 Deployed to staging by @marcaaron in version: 1.1.41-0 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	failure ❌

mvtglobally · 2022-03-07T03:54:09Z

Team is having a bit hard time validating this PR on Desktop as it requires device language change and introducing navigation issues to flip language back as options shift.
We validated Web/mWeb/IOS and Android. It would be great if someone can validate internally Desktop

ahmdshrif · 2022-03-07T08:11:35Z

#7922 (comment) same suggestion here.

francoisl · 2022-03-08T17:57:14Z

Hi @mvtglobally, were you able to test with the suggestion here?

mvtglobally · 2022-03-08T20:06:44Z

@francoisl yes, checked off both PRs

OSBotify · 2022-03-11T00:04:03Z

🚀 Deployed to production by @francoisl in version: 1.1.41-6 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

mollfpr · 2023-01-27T14:50:20Z

Coming from #13802, there's a code that is causing that regression. Setting textAlign props to the TextInput makes the ... missing on iOS. Explanation proposal here. Thanks!

parasharrajat · 2023-01-27T15:20:57Z

TextAlign:left was necessary for forcing RTL mode. Why do we need ... on the text input if the text is long? It should be scrollable not clipped with an ellipsis.

sakluger · 2023-01-30T17:18:24Z

@parasharrajat the fix only adds the ... on iOS, where that is the standard pattern for text longer than the field. All other platforms (web, desktop app, android) all have scrollable text.

ahmdshrif marked this pull request as ready for review February 7, 2022 00:21

ahmdshrif requested a review from a team as a code owner February 7, 2022 00:21

botify requested review from marcaaron and parasharrajat February 7, 2022 00:21

MelvinBot requested review from marcaaron and parasharrajat and removed request for a team, parasharrajat and marcaaron February 7, 2022 00:21

parasharrajat suggested changes Feb 9, 2022

View reviewed changes

ahmdshrif requested a review from parasharrajat February 9, 2022 18:21

parasharrajat suggested changes Feb 9, 2022

View reviewed changes

index.js Outdated Show resolved Hide resolved

src/components/TextInput/TextInputLabel/TextInputLabelPropTypes.js Outdated Show resolved Hide resolved

ahmdshrif requested a review from parasharrajat February 9, 2022 19:49

ahmdshrif force-pushed the fix/rtl-mode branch from 1af91c7 to 18b2a93 Compare February 9, 2022 21:14

parasharrajat suggested changes Feb 10, 2022

View reviewed changes

parasharrajat suggested changes Feb 24, 2022

View reviewed changes

src/components/BaseInput/index.js Outdated Show resolved Hide resolved

src/components/BaseInput/index.js Outdated Show resolved Hide resolved

ahmdshrif and others added 2 commits February 24, 2022 17:18

Update src/components/BaseInput/index.js

60d020c

Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>

use underscore insted of typeof

23fc4a1

parasharrajat reviewed Feb 24, 2022

View reviewed changes

parasharrajat suggested changes Feb 24, 2022

View reviewed changes

parasharrajat previously approved these changes Feb 24, 2022

View reviewed changes

ahmdshrif requested a review from marcaaron February 24, 2022 19:37

marcaaron requested changes Feb 24, 2022

View reviewed changes

rename baseinput to RNTextInput

a81e2ea

ahmdshrif dismissed parasharrajat’s stale review via a81e2ea February 25, 2022 08:33

ahmdshrif requested a review from marcaaron February 25, 2022 08:33

marcaaron approved these changes Feb 25, 2022

View reviewed changes

marcaaron merged commit 2be5a0e into Expensify:main Feb 25, 2022

OSBotify mentioned this pull request Mar 1, 2022

Deploy Checklist: New Expensify 2022-03-01 #7954

Closed

68 tasks

botify mentioned this pull request Mar 10, 2022

[HOLD for payment 2022-03-17] App changes positions of Ui elements when phone language RTL - Reported by Ahmed sherif #7476

Closed

Stutikuls mentioned this pull request Mar 16, 2022

Implement infinite sessions #7

Merged

mollfpr mentioned this pull request Jan 26, 2023

[HOLD for payment 2023-01-25] [$1000] Long text strings disappear from single-line text fields when text is too long #13802

Closed

1 task

Fix: App changes positions of Ui elements when phone language RTL #7594

Fix: App changes positions of Ui elements when phone language RTL #7594

Conversation

ahmdshrif commented Feb 6, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

ahmdshrif commented Feb 7, 2022 • edited Loading

parasharrajat commented Feb 7, 2022 • edited Loading

marcaaron commented Feb 7, 2022

ahmdshrif commented Feb 7, 2022 • edited Loading

parasharrajat commented Feb 7, 2022

ahmdshrif commented Feb 7, 2022

ahmdshrif commented Feb 7, 2022

parasharrajat commented Feb 7, 2022 • edited Loading

ahmdshrif commented Feb 7, 2022

parasharrajat commented Feb 8, 2022 • edited Loading

parasharrajat commented Feb 8, 2022

ahmdshrif commented Feb 9, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

ahmdshrif commented Feb 9, 2022

ahmdshrif commented Feb 9, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

ahmdshrif commented Feb 9, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

parasharrajat left a comment

Choose a reason for hiding this comment

ahmdshrif commented Feb 24, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

parasharrajat left a comment

Choose a reason for hiding this comment

ahmdshrif commented Feb 24, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

marcaaron Feb 24, 2022

Choose a reason for hiding this comment

ahmdshrif Feb 25, 2022

Choose a reason for hiding this comment

marcaaron Feb 25, 2022

Choose a reason for hiding this comment

ahmdshrif Feb 25, 2022

Choose a reason for hiding this comment

OSBotify commented Feb 25, 2022

OSBotify commented Mar 1, 2022

OSBotify commented Mar 2, 2022

mvtglobally commented Mar 7, 2022

ahmdshrif commented Mar 7, 2022

francoisl commented Mar 8, 2022

mvtglobally commented Mar 8, 2022

OSBotify commented Mar 11, 2022

mollfpr commented Jan 27, 2023

parasharrajat commented Jan 27, 2023

sakluger commented Jan 30, 2023

ahmdshrif commented Feb 6, 2022 •

edited

Loading

ahmdshrif commented Feb 7, 2022 •

edited

Loading

parasharrajat commented Feb 7, 2022 •

edited

Loading

ahmdshrif commented Feb 7, 2022 •

edited

Loading

parasharrajat commented Feb 7, 2022 •

edited

Loading

parasharrajat commented Feb 8, 2022 •

edited

Loading