fix(iOS): restore native behavior of auto shortening back button title #2105
Conversation
| if (config.isBackTitleVisible) { | ||
| if (config.backTitleFontFamily || config.backTitleFontSize) { | ||
| if ((config.backTitleFontFamily && | ||
| ![config.backTitleFontFamily isEqual:@"System"]) || |
There was a problem hiding this comment.
Why do we need to check the font family here?
There was a problem hiding this comment.
It's because due to the current implementation of @react-navigation/native-stack, backTitleFontFamily will always be assigned to the font family of fonts.regular, which will be "System" if the default theme is used.
It seems that the "System" font family is already the default on iOS, as setting the font family to "System" does not change the appearance, but in this case, will make config.backTitleFontFamily always resolve to YES.
There was a problem hiding this comment.
I see. Do you know, if we could rely on the native component that represents the title of back button, instead of comparing config.backTitleFontFamily?
There was a problem hiding this comment.
Thanks for the PR, this is great!
Can we have this comment in the code please? @zetavg
There was a problem hiding this comment.
Thanks for the comment @tboba, but I'm sorry that due to my limited knowledge, I can't quite understand what you mean. Do you suggest we can handle this in something like the getter/setter of RNSScreenStackHeaderConfig, or the JS code in react-navigation?
@kkafar Sure, I'll add comments to explain this besides related code once we finalize the solution.
There was a problem hiding this comment.
@zetavg I'm wondering if it would be possible to get title component of back title and check if it has default font set, but after further reflections I'm afraid this could lead to some flickers (setting the font and other properties -> checking the font of the native component -> setting system back 😄), so I guess we can stay with the current solution.
There was a problem hiding this comment.
Thanks for explaining, I think I got your point! I do think the current solution of comparing the font against "System" isn't perfect, one way is that there may be different ways to use the same system font (maybe ".AppleSystemUIFont"? I'm not sure), and if react-navigation changes to also assign a default value to config.backTitleFontSize this will break again.
Your solution inspired me to think of a possible way to get this native behavior to work while still customizing the back button (such as changing the font or customizing the shortened "Back" text) - measure the width of the headerTitle and headerRight to see if there's enough space on layout, and replace the backButtonTitle with the shortened text or hide it. Well, flickering may be a problem.
…st "System" is needed
|
Comment added! Please check if it's suitable 😃 |
|
Yeah, I think for now it's a pass 🎉 |
Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
|
Hi @kkafar, just a ping to let you know I have applied your suggestion (thanks for that!) for the change you requested. If there's anything I can do to get this PR merged, please let me know. |
| @@ -535,9 +544,17 @@ + (void)updateViewController:(UIViewController *)vc | |||
| // When backBarButtonItem's title is null, back menu will use value | |||
| // of backButtonTitle | |||
| [backBarButtonItem setTitle:nil]; | |||
| isBackButtonCustomized = YES; | |||
There was a problem hiding this comment.
Hi @zetavg, I know that this PR is already merged, but I wonder why do you set isBackButtonCustomized to true here. My understanding is that this else block is for the case when we hide back button, but want to preserve it in back menu. So, by default when we have navigationItem hidden we don't display anything here - nothing is customized, when we want to custom hide it from back manu then we set config.disableBackButtonMenu which is in initial condition anyway.
There was a problem hiding this comment.
Quick reply - I think it's because this isBackButtonCustomized is used to determine whether we need to assign the backBarButtonItem to prevItem.backBarButtonItem later on line 556.
Since backBarButtonItem has been changed here ([backBarButtonItem setTitle:nil];), my understanding is that the changed backBarButtonItem has to be assigned to prevItem.backBarButtonItem for the changes to take effect. So it should be considered customized - or changed. Maybe it's better to rename isBackButtonCustomized to isBackBarButtonItemChanged so that it will be more clear (or use another way to track if we have changed anything on backBarButtonItem).
My guess is that if we do not set isBackButtonCustomized to true here, the back title won't be hidden as expected. I may need to verify it though.
There was a problem hiding this comment.
Maybe it's better to rename
isBackButtonCustomizedtoisBackBarButtonItemChanged...
Oh, I see that you had already done this in #2123. 🎉
## Description ~This PR improves upon #2105. #2105 allowed to use iOS 14 default back button behavior when label is not provided. This PR allows to modify the behavior by allowing to provide UINavigationButtonBackButtonDisplayMode and enables it for custom text (without style modifications). The main problem is that we used to provide backButtonItem in most of the cases which [disables](https://developer.apple.com/documentation/uikit/uinavigationitem/3656350-backbuttondisplaymode) backButtonDisplayMode.~ This PR adds possibility to customize default behavior of back button using `backButtonDisplayMode` ([UINavigationBackButtonDisplayMode](https://developer.apple.com/documentation/uikit/uinavigationitem/backbuttondisplaymode)) for iOS. :warning: **This modifies only default back button**, when any customization is added (including headerBackTitle) in native part we create custom `RNSUIBarButtonItem` and set it as `backButtonItem`, which [disables](https://developer.apple.com/documentation/uikit/uinavigationitem/3656350-backbuttondisplaymode) `backButtonDisplayMode` behavior. I tried to make it work together with custom label (`headerBackTitle`) using `prevItem.backButtonTitle`, but due to iOS limitations it is not viable option. It influences also back button menu - changes the label of previous screen - which is not the behavior we want. To sum up, `backButtonDisplayMode` work when none of: - `headerBackTitleStyle.fontFamily` - `headerBackTitleStyle.fontSize` - `headerBackTitle` - `disableBackButtonMenu` are set. ## Screenshots / GIFs |Paper|Fabric| |-|-| |<video src="https://github.com/software-mansion/react-native-screens/assets/11800297/c6aa7697-4331-4cb4-a81d-7f77f128513d" />|<video src="https://github.com/software-mansion/react-native-screens/assets/11800297/fa0edd92-1aa2-45e5-a466-516c0ec120d2" />| <details> <summary>Example component used in tests:</summary> ```jsx import * as React from 'react'; import { Button, View, Text, StyleSheet } from 'react-native'; import { NavigationContainer, ParamListBase } from '@react-navigation/native'; import { createNativeStackNavigator } from '@react-navigation/native-stack'; import { NativeStackNavigationProp } from '@react-navigation/native-stack'; const Stack = createNativeStackNavigator(); type NavProp = { navigation: NativeStackNavigationProp<ParamListBase>; }; export default function App() { return ( <NavigationContainer> <Stack.Navigator> <Stack.Screen name="screenA" component={ScreenA} options={{ headerTitle: 'A: Home' }} /> <Stack.Screen name="screenB" component={ScreenB} options={{ headerTitle: 'B: default', backButtonDisplayMode: 'default', }} /> <Stack.Screen name="screenC" component={ScreenC} options={{ headerTitle: 'C: generic', backButtonDisplayMode: 'generic', }} /> <Stack.Screen name="screenD" component={ScreenD} options={{ headerTitle: 'D: minimal', backButtonDisplayMode: 'minimal', }} /> <Stack.Screen name="screenE" component={ScreenE} options={{ headerTitle: 'E: custom', headerBackTitle: 'Back Title', backButtonDisplayMode: 'minimal', }} /> </Stack.Navigator> </NavigationContainer> ); } const ScreenA = ({ navigation }: NavProp) => ( <View style={styles.container}> <Text>Screen A</Text> <Button onPress={() => navigation.navigate('screenB')} title="Go to screen B" /> </View> ); const ScreenB = ({ navigation }: NavProp) => ( <View style={styles.container}> <Text>Screen B</Text> <Text>backButtonDisplayMode: default</Text> <Button onPress={() => navigation.navigate('screenC')} title="Go to screen C" /> </View> ); const ScreenC = ({ navigation }: NavProp) => ( <View style={{ flex: 1, paddingTop: 50 }}> <Text>Screen C</Text> <Text>backButtonDisplayMode: generic</Text> <Button onPress={() => navigation.navigate('screenD')} title="Go to screen D" /> </View> ); const ScreenD = ({ navigation }: NavProp) => ( <View style={styles.container}> <Text>Screen D</Text> <Text>backButtonDisplayMode: minimal</Text> <Button onPress={() => navigation.navigate('screenE')} title="Go to screen E" /> </View> ); const ScreenE = (_props: NavProp) => ( <View style={styles.container}> <Text>Screen E</Text> <Text>backButtonDisplayMode omitted because of the headerBackTitle</Text> </View> ); const styles = StyleSheet.create({ container: { flex: 1, alignItems: 'center', justifyContent: 'space-around' }, }); ``` </details> ## Checklist - [x] Included code example that can be used to test this change - [x] Updated TS types - [x] Updated documentation: <!-- For adding new props to native-stack --> - [x] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [x] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [x] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [x] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [x] Ensured that CI passes Tested #1864: Paper ✅ Fabric ✅ Tested #1646: Paper ❌ Fabric ❌ - but it does not work on main too, could now be achieved using `backButtonDisplayMode: ‘minimal’` --------- Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
- Fixes: Title truncated on NewNominationScreen by long back button title instead of "Back" - Introduced during the RN73 upgrade - software-mansion/react-native-screens#1589 - software-mansion/react-native-screens#2105 - https://github.com/software-mansion/react-native-screens/releases/tag/3.32.0
- Fixes: Title truncated on NewNominationScreen by long back button title instead of "Back" - Introduced during the RN73 upgrade - software-mansion/react-native-screens#1589 - software-mansion/react-native-screens#2105 - https://github.com/software-mansion/react-native-screens/releases/tag/3.32.0
Description
Restore the iOS native behavior of automatically shorting the title of the header back button to "Back" if there is not enough space, which is documented here1 but does not behave as expected since v3.21.
Fixes #1589.
Changes
backBarButtonItemonly if actual customizations of the back button are being made.Screenshots / GIFs
Test code and steps to reproduce
Checklist
Footnotes
According to the document, 'On iOS this includes a label next to the button, which shows the title of the previous screen when the title fits in the available space, otherwise it says "Back".' ↩