Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

headerLargeTitle & headerSearchBarOptions Doesn't Support RTL #1884

Closed
Wahsner opened this issue Sep 7, 2023 · 11 comments · Fixed by #1895 or #2185
Closed

headerLargeTitle & headerSearchBarOptions Doesn't Support RTL #1884

Wahsner opened this issue Sep 7, 2023 · 11 comments · Fixed by #1895 or #2185
Assignees
Labels
Missing repro This issue need minimum repro scenario Platform: iOS This issue is specific to iOS

Comments

@Wahsner
Copy link

Wahsner commented Sep 7, 2023

Description

When RTL is enabled, the headerLargeTitle & headerSearchBarOptions don't honour the text direction. Instead they remain LTR as shown in the below screen cuttings.

  • Restarting the app between changing text direction makes no difference.
  • The fact the text itself isn't a RTL language makes no difference.
  • Having the device in a RTL language makes no difference.
  • Doesn't matter if you're using Paper or Fabric or the new/old arch.
  • Doesn't matter if you're running in debug or release.
  • Doesn't matter if you're running on a real or sim device.

headerLargeTitle
image
headerSearchBarOptions(focused)
image

I would expect them to be positioned right as shown below (taken from the contacts app)

Large Title
image
Search bar(focused)
image

Steps to reproduce

  1. Set the options direction: 'rtl'
  2. Not sure if required, but Set I18nManager.forceRTL(true); in the App.js too
  3. Restart the app by closing it completely from background and re-launching

Expected

  • For text direction to be RTL for headerLargeTitle & headerSearchBarOptions

Actual

  • headerLargeTitle & headerSearchBarOptions are still LTR

Snack or a link to a repository

https://github.com/0xRenshaw/react-native-screens/tree/main/FabricExample

Screens version

3.25.0

React Native version

0.72.4

Platforms

iOS

JavaScript runtime

None

Workflow

React Native (without Expo)

Architecture

Paper (Old Architecture)

Build type

Release mode

Device

Real device

Device model

No response

Acknowledgements

Yes

@github-actions github-actions bot added Platform: iOS This issue is specific to iOS Missing repro This issue need minimum repro scenario Repro provided A reproduction with a snack or repo is provided and removed Missing repro This issue need minimum repro scenario labels Sep 7, 2023
@tboba tboba self-assigned this Sep 19, 2023
@tboba
Copy link
Member

tboba commented Sep 20, 2023

Hi @0xRenshaw, thank you for reporting this issue!
I've managed to reproduce this problem - I'll open a PR that solves it shortly.

Screenshot 2023-09-20 at 10 12 25

tboba added a commit that referenced this issue Oct 3, 2023
## Description

In the current state, the RTL mode didn't work correctly for large
headers and search bar - even if user sets RTL mode in their app, the
content of the large header and search was still on the left side. Also,
what I've found, the icon of the back button was still pointing to the
left, even if the back button was on the right side of the screen.

Fixes #1884.

## Changes

- Added a workaround for pointing arrow to the left - unfortunately I
couldn't find a better solution for changing the direction of the arrow.
- Changed the direction of the contents in the navigation bar when user
is in RTL mode.

## Screenshots / GIFs

### Before

<img width="365" alt="image"
src="https://github.com/software-mansion/react-native-screens/assets/23281839/c2f6fd37-3b00-4ec4-aec6-ba8e55566631">
<br/>

<img width="383" alt="image"
src="https://github.com/software-mansion/react-native-screens/assets/23281839/8504ed8c-1c85-4c2c-bcd9-f1419a2d60d7">

### After

<img width="364" alt="image"
src="https://github.com/software-mansion/react-native-screens/assets/23281839/66410e3e-0dd7-4ab2-a9b6-5860d56e32bf"><br/>

<img width="383" alt="Screenshot 2023-09-20 at 10 27 51"
src="https://github.com/software-mansion/react-native-screens/assets/23281839/4239cad9-e2d9-4bb3-b46f-434487bdf104">



## Test code and steps to reproduce

1. Launch example application from `Example` directory.
2. Switch `Right to left` option to `true`
3. Check `Header Options` and `Search bar` tabs to see the difference.
4. You can also change `headerHideBackButton` and `headerLargeTitle`
options inside the stack's navigator and go to `Other Searchbar example`
to check how large header behaves with the search bar.

## Checklist

- [ ] Ensured that CI passes
@moedeveloper
Copy link

what is the fix, i am still having the same issue ?

@Wahsner
Copy link
Author

Wahsner commented Oct 12, 2023

what is the fix, i am still having the same issue ?

It has been merged to master. There hasn't yet been a release.

@tboba
Copy link
Member

tboba commented Oct 17, 2023

Hi @0xRenshaw @moedeveloper, there's a new version of react-native-screens: 3.26.0 which has this change included. Check it out! If you find something wrong related to the newest version (this change is still buggy or doesn't work for you) let us know 🎉

@STonkoshkur
Copy link

@tboba the back button direction is still incorrect in RTL for iOS. I'm running the example application from Example directory using Xcode 15.2

image

@tboba
Copy link
Member

tboba commented Feb 16, 2024

@STonkoshkur interesting, do you have some native apps that have RTL mode enabled? I just wanted to see how the back button should look like. In my case, I have the same back button as you, but if I unhide the default navigation bar of the navigation controller, it looks completely the same 🤔

image

@tboba
Copy link
Member

tboba commented Feb 16, 2024

Reopening the issue, since something has broke in iOS 17.2 that has broke the back button in RTL mode.

Expected behavior:
image

@tboba tboba reopened this Feb 16, 2024
@github-actions github-actions bot added the Missing repro This issue need minimum repro scenario label Feb 16, 2024
Copy link

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

@tboba
Copy link
Member

tboba commented Feb 16, 2024

I believe that potentially the bug with back button might occur also in iOS native apps. I've found on Apple forums that someone has the same problem - https://developer.apple.com/forums/thread/737855.
@STonkoshkur could you check if on the non-system native apps the issue with back button is the same?

@serhiitonkoshkur
Copy link

@tboba I've checked several non-system iOS apps and the back button is working fine (iOS 17.2.1)

@alduzy
Copy link
Member

alduzy commented Jun 12, 2024

I can confirm the back arrow faces correct direction after changing the application's CFBundleDevelopmentRegion.
Looks as if the I18nManager.forceRTL(true) does not force the change.

developmentRegion = ar developmentRegion = en
Screenshot 2024-06-12 at 12 41 14 Screenshot 2024-06-12 at 12 42 33
Screenshot 2024-06-12 at 12 41 27 Screenshot 2024-06-12 at 12 42 46

It's worth noticing that the Text element's alignment is not changing properly - only flexDirection of View element changes:
Screenshot 2024-06-12 at 12 33 10

alduzy added a commit that referenced this issue Jun 28, 2024
…ling in header (#2185)

## Description

This PR intents to fix back button direction in RTL mode forced by
I18nManager for testing purposes on iOS.

When the application's language is changed using the
`CFBundleDevelopmentRegion` the system is well aware of that, passes the
information to React Native and Yoga. The result is a fully LTR / RTL
compliant application.

However, when the `RTL` mode is set using the React Native's
`I18nManager.forceRTL()` method (for testing purposes I believe) The
change is applied to react views only and the system itself does not
change it's direction.
In such case the only way for the native elements, such as
`navigationBar` to adapt to the direction set by `I18nManager` is by
manually applying the `semanticContentAttribute` based on `direction`
prop passed by `react-navigation`.

> [!note]
In case there is another native `navigationBar` element apart from the
one coming from `react-native-screens`, this change may override it's
direction.

Tested successfully on iOS 17.5 and 16.4

Fixes #1884 .

## Changes

- removed faulty workaround
- set correct semanticContentAttribute for navigationBar and it's
contents
- refactored how the searchbar direction is set

## Screenshots / GIFs

### Before

![image](https://github.com/software-mansion/react-native-screens/assets/91994767/1c67071d-92e4-4966-9ad8-362c54c4435b)

### After

![image](https://github.com/software-mansion/react-native-screens/assets/91994767/5cf40cf1-b762-4720-af33-0f939adac7c5)

## Test code and steps to reproduce

- play around with LTR and RTL mode in the example app

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Updated documentation: <!-- For adding new props to native-stack
-->
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
- [ ] Ensured that CI passes

## Short story on how RTL management works in RN apps on iOS by @kkafar

> [!note]
> Basically we have two RTL management systems: native one &
ReactNative's I18nManager.
>
> Native one works most likely by applying semantic content attribute to
all the views / classes and impacting layout constraints, thus whole
native application is laid out properly.
> We can impact RTL on natively managed views by:
> * setting semantic content attributes for particular view
> * [setting semantic content attributes for particular class through
appearance
proxy](https://developer.apple.com/documentation/uikit/uiappearance?language=objc)
> * [setting semantic content attributes for particular class contained
in instances of another
class](https://developer.apple.com/documentation/uikit/uiappearance/1615006-appearancewhencontainedin?language=objc)
> 
> Topic's to research:
> * how parent's semantic content attribute (set directly on given view)
does affect subviews
> * exact semantics of using appearance proxy object, their priority and
resolve order.
> 
> 
> React Native has notion of internationalisation manager (I18n) which
works as follows:
> 
> * The RTL is allowed by default.
> * If RTL is not forced from JS, and allowed, then system language
writing direction is resolved based on system calls and such obtained
interface direction is passed down to Yoga *impacting all views that are
laid out by Yoga*, but not purely native ones such as our header. Thus
leading to conformity between system options and RN app options.
> * If RTL is forced from JS and allowed, them Yoga managed views are
laid out in RTL mode, while system managed ones respect the system
settings.
> * ForceRTL / AllowRTL flags are stored in `NSUserDefaults` application
persistent storage, thus living through application restarts. These
flags are RN specific, and system has no knowledge of them.
> 
> React Navigation takes the value from RN I18n system and passes it
down to our screen / stack / header components.

---------

Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Missing repro This issue need minimum repro scenario Platform: iOS This issue is specific to iOS
Projects
None yet
6 participants