software-mansion / react-native-screens

Native navigation primitives for your React Native app.
https://docs.swmansion.com/react-native-screens/
MIT License
3.01k stars 510 forks source link

Header back button does not shorten to 'Back' on iOS if there's not enough space #1589

Closed DrOverbuild closed 4 months ago

DrOverbuild commented 2 years ago

Description

According to the documentation, the back button title is supposed to change to "Back" if there's not enough space for the full title of the previous screen. That is not what I'm experiencing. In the GIF below you can see that instead of changing to "Back" it will truncate the header title, and if it's long enough, the back button will use the entire header width, hiding the title.

Title of back button is long enough to truncate the header title

Title of back button is long enough to remove the header title and back button icon

Steps to reproduce

  1. Create a screen with a long enough title
  2. From the newly created screen, navigate to another screen with default screen options
  3. Observe the long title of the back button

Snack or a link to a repository

https://github.com/DrOverbuild/BackButtonRepro

Screens version

3.17.0

React Native version

0.70.0

Platforms

iOS

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Paper (Old Architecture)

Build type

Debug mode

Device

iOS simulator

Device model

iPhone 13 (iOS 15.5)

Acknowledgements

Yes

DrOverbuild commented 2 years ago

After a whole afternoon of investigation I have found the issue. There is a system issue where the back button does not change to "Back" if the navigation item's backButtonTitle is customized or the navigation item has a custom backBarButtonItem.

Even if there is no headerBackTitle prop set, in this block of code, config.backTitle is set to the value of the title of the previous screen when the screen options are updated for screen 2. Therefore, a custom RNSUIBarButtonItem is created and the navigation item's backBarButtonItem is set to that.

I have found that this is only caused when I use createNativeStackNavigator from @react-navigation/native-stack, which is the recommended way to import. If I import createNativeStackNavigator from react-native-screens/native-stack, the correct behavior is achieved if I don't customize the style of the back button.

I think we can still take a look at keeping this behavior if the style is changed.

We may want to look at using UINavigationBarAppearance for iOS 13.0 and newer to change the style of the back button. With the following snippet I was able change the font of the back button while maintaining system behavior:

NSMutableDictionary *attrs = [NSMutableDictionary new];
attrs[NSFontAttributeName] = [UIFont fontWithName:@"AvenirNextCondensed-DemiBoldItalic" size:17];
UINavigationBarAppearance *appearance = [[UINavigationBarAppearance alloc] init];
appearance.backButtonAppearance.normal.titleTextAttributes = attrs;
navctr.navigationBar.scrollEdgeAppearance = appearance;
navctr.navigationBar.standardAppearance = appearance;

We'd probably want to combine something like that to lines 505-518.

Not sure if there's a solution for this for versions before iOS 13.0 or when the long press menu needs to be disabled.

I'd be happy to submit a PR but I wanted to discuss this a little first.

h-five-e commented 1 year ago

This bug is (still) fixed in v3.20, but it breaks again in v3.21 and newer.

I have provided a repro here: https://github.com/h-five-e/repro-navigation-Ios-back-button-shows-long-title Commit with just the relevant changes from a new app: https://github.com/h-five-e/repro-navigation-Ios-back-button-shows-long-title/commit/51372e77470234efc7a8926ed63ab84d9e42d450

image image image
h-five-e commented 1 year ago

Hey @kkafar, I don't mean to badger you but given we talked about it here I wondered if you've seen this. Lmk if I should create a new issue instead!

zetavg commented 5 months ago

I did some experimentation, and it seems this issue still exists with @react-navigation/native-stack v7 and "react-native-screens": "3.30.1" without RCT_NEW_ARCH_ENABLED.

I tested a few more versions and can confirm that it's working on v3.20 but broken in v3.21 and newer as @h-five-e stated, and the issue is still valid on 3.31.0-rc.1 even with createNativeStackNavigator imported from react-native-screens/native-stack.

Considered as Working ✅ Working 1 Working 2
Considered as Broken ❌ Broken 1
zetavg commented 5 months ago

After some investigation of the code, I think the cause is that in v3.21, the backBarButtonItem of UINavigationItem is always assigned a custom UIBarButtonItem, regardless of whether the back button is actually being customized, and assigning a custom UIBarButtonItem will remove the native behavior of automatically shortening the title to "Back" or hide the back title if there's not enough space.

As the desired behavior can be restored with this patch:

diff --git a/ios/RNSScreenStackHeaderConfig.mm b/ios/RNSScreenStackHeaderConfig.mm
index f7ec74678166f485a58ff3b8053a63befad14cfe..92adfb2874e027686c9bea82e5107c568bf5816f 100644
--- a/ios/RNSScreenStackHeaderConfig.mm
+++ b/ios/RNSScreenStackHeaderConfig.mm
@@ -511,8 +511,13 @@ namespace react = facebook::react;
                                                                              action:nil];
   [backBarButtonItem setMenuHidden:config.disableBackButtonMenu];

+  auto isBackButtonCustomized = !isBackTitleBlank || config.disableBackButtonMenu || NO;
+
   if (config.isBackTitleVisible) {
-    if (config.backTitleFontFamily || config.backTitleFontSize) {
+    if ((config.backTitleFontFamily &&
+         ![config.backTitleFontFamily isEqual:@"System"]) ||
+        config.backTitleFontSize) {
+      isBackButtonCustomized = YES;
       NSMutableDictionary *attrs = [NSMutableDictionary new];
       NSNumber *size = config.backTitleFontSize ?: @17;
       if (config.backTitleFontFamily) {
@@ -535,9 +540,17 @@ namespace react = facebook::react;
     // When backBarButtonItem's title is null, back menu will use value
     // of backButtonTitle
     [backBarButtonItem setTitle:nil];
+    isBackButtonCustomized = YES;
     prevItem.backButtonTitle = resolvedBackTitle;
   }
-  prevItem.backBarButtonItem = backBarButtonItem;
+
+  // Prevent unnecessary assignment of backBarButtonItem if it is not customized,
+  // as assigning one will override the native behavior of automatically shortening
+  // the title to "Back" or hide the back title if there's not enough space.
+  // See: https://github.com/software-mansion/react-native-screens/issues/1589
+  if (isBackButtonCustomized) {
+    prevItem.backBarButtonItem = backBarButtonItem;
+  }

   if (@available(iOS 11.0, *)) {
     if (config.largeTitle) {
JustJoostNL commented 3 weeks ago

Hey!

Great that this problem is solved, but is there a way to override/make sure that the back text even show's if that results in shortening?

kkafar commented 3 weeks ago

@JustJoostNL could you describe a bit more expressively what case exactly do you mean here?

JustJoostNL commented 3 weeks ago

@kkafar In my app, I always want the back text to show (the title of the previous page) whether it fits or not.

kkafar commented 3 weeks ago

I don't think this is possible right now, after changes from #2105 were applied. It was considered as breaking the native behaviour and we decided to go against it. This is not the way iOS implements this & the idea of native header is to follow iOS beahavior here. You can try experimenting with disabling back button & putting your custom header left there or implement whole custom header.