feat: expose tabBarGap option in material top tabs #10984
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- @react-navigation/bottom-tabs@6.0.0-next.20 - @react-navigation/core@6.0.0-next.15 - @react-navigation/devtools@6.0.0-next.16 - @react-navigation/drawer@6.0.0-next.19 - @react-navigation/elements@1.0.0-next.19 - flipper-plugin-react-navigation@1.3.0 - @react-navigation/material-bottom-tabs@6.0.0-next.16 - @react-navigation/material-top-tabs@6.0.0-next.16 - @react-navigation/native-stack@6.0.0-next.9 - @react-navigation/native@6.0.0-next.15 - @react-navigation/routers@6.0.0-next.6 - @react-navigation/stack@6.0.0-next.27
I noticed that accessing `closing` through `cardStyleInterpolator` would always return an Animated node with value 0. It looks like it isn't being updated anywhere, so I added it to the `animate` method. I am using this functionality to allow screens to have different in and out transitions. On a side note, I feel like this would be more useful as a boolean, instead of an Animated value. Co-authored-by: Michael Ru <michaelru@abridge.com>
…gation#9688) Problem: When using nested navigators, unmounts cause race cleanup races. Imagine following hierarchy (from tree root downwards, parent to children): TabNavigator (1) [renders useNavigationBuilder] SceneView (from TabNavigator) StackNavigators (N) [each renders useNavigationBuilder] SceneView (from StackNavigator) Now lets test following flow: 1. Mount above navigators with given navigation params (e.g. navigation for unauthenticated users) 2. Unmount all navigators (e.g. during login process) 3. Mount above navigation with different navigation params than in 1) (e.g. navigation for authenticated users) What you'll observe, there will be old navigation params preserved in 3) coming from 1). Source of problem: BaseNavigationContainer holds global navigation state, exposes API to modify it via NavigationStateContext. When useNavigationBuilder unmounts, it attempts to clear navigation state. (see cleanup effect in useNavigationBuilder.tsx). (I) First clear occurs in TabNavigator's effect, which successfully clears BaseNavigationContainer's state (sets it to undefined). (II) Second clear comes from StackNavigator unmount. It's useNavigationBuilder cleanup effect also calls NavigationStateContext.setState(undefined). But this time - we meet SceneView as closest NavigationStateContext.Provider. SceneView attempts to merge state change with current navigation state, which is reasonable. But current navigation state should be already undefined... It is, but: ``` [useNavigationBuilder.tsx] const getState = React.useCallback((): State => { const currentState = getCurrentState(); return isStateInitialized(currentState) ? (currentState as State) : (initializedStateRef.current as State); }, [getCurrentState, isStateInitialized]); ``` "undefined" state is treated is non-initialized state, and freshly computed state (initializedStateRef.current) is returned instead. SceneView does merge this old state with `undefined` value, and passes to BaseNavigationContainer. Now we have some legacy global state, despite all navigators being unmounted. After mounting navigators again (3), we can observe old params being restored. These params might come e.g. from old `initialParams` prop (from 1)). Solution: Do not propagate `setState` upwards in `useNavigationBuilder` after state cleanup. This way we'll omit such races.
- @react-navigation/bottom-tabs@6.0.0-next.21 - @react-navigation/core@6.0.0-next.16 - @react-navigation/devtools@6.0.0-next.17 - @react-navigation/drawer@6.0.0-next.20 - @react-navigation/elements@1.0.0-next.20 - flipper-plugin-react-navigation@1.3.1 - @react-navigation/material-bottom-tabs@6.0.0-next.17 - @react-navigation/material-top-tabs@6.0.0-next.17 - @react-navigation/native-stack@6.0.0-next.10 - @react-navigation/native@6.0.0-next.16 - @react-navigation/stack@6.0.0-next.28
This fixes an issue where the new actions could bring back the params even after it was reset
- @react-navigation/bottom-tabs@6.0.0-next.22 - @react-navigation/core@6.0.0-next.17 - @react-navigation/devtools@6.0.0-next.18 - @react-navigation/drawer@6.0.0-next.21 - @react-navigation/elements@1.0.0-next.21 - flipper-plugin-react-navigation@1.3.2 - @react-navigation/material-bottom-tabs@6.0.0-next.18 - @react-navigation/material-top-tabs@6.0.0-next.18 - @react-navigation/native-stack@6.0.0-next.11 - @react-navigation/native@6.0.0-next.17 - @react-navigation/stack@6.0.0-next.29
- @react-navigation/devtools@6.0.0-next.19
This is useful for libraries like `expo-auth-session` which also use links for authentication.
Usage:
```js
const linking = {
prefixes: ['myapp://'],
filter: (url) => !url.includes('+expo-auth-session'),
};
```
Unfortunately the changes from react-navigation#10761 caused an issue I reported on the react-native-screens repo. (See software-mansion/react-native-screens#1589) Essentially, the documentation states that per native behavior, the back button on iOS will automatically use the title of the previous screen or "Back" if there's not enough space. However, with the latest version of react-navigation, the back button was the title of the previous screen no matter what. In some cases, this caused the title of the current screen to be completely pushed off the screen. In UIKit this happens when the title of the back button is deliberately set, or when a custom UIBarButtonItem is set as the back button. `react-native-screens` creates a custom UIBarButtonItem when the header config component has a value defined for `headerBackTitle`. So by leaving headerBackTitle undefined here, we let the system automatically determine what to set as the back button title. In my testing, system will choose the correct value even if the previous screen's `title` or `headerTitle` is set differently from the route name.
In order to fully support Fabric in @react-navigation/stack we need to remove the use of `setNativeProps` as stated in the official migrating documentation: [Moving setNativeProps to state](https://reactnative.dev/docs/next/new-architecture-library-intro#moving-setnativeprops-to-state) In our case, `setNativeProps` is used for disabling `pointerEvents` during the transition of the screen. I've prepared a reproduction to show a successful migration from `setNativeProps` to the `state`. Fixes react-navigation#10720
- @react-navigation/bottom-tabs@6.4.0 - @react-navigation/core@6.4.0 - @react-navigation/devtools@6.0.10 - @react-navigation/drawer@6.5.0 - @react-navigation/elements@1.3.6 - flipper-plugin-react-navigation@1.3.14 - @react-navigation/material-bottom-tabs@6.2.4 - @react-navigation/material-top-tabs@6.2.4 - @react-navigation/native-stack@6.9.0 - @react-navigation/native@6.0.13 - @react-navigation/routers@6.1.3 - @react-navigation/stack@6.3.0
This reverts commit 495b28f.
- @react-navigation/stack@6.3.1
…date` (react-navigation#10871) Changes introduced in react-navigation#10767 created a regression reported in react-navigation#10856 which made it impossible to close a modal with a gesture. As we want to change pointer-events on Card in componentDidUpdate previous use of setNativeProps in this context made a lot of sense. Since setNativeProps is going to be removed in the new architecture we had to migrate it to the component state as stated in the migration guide. This PR adds an additional `closing !== prevProps.closing` check on top of react-navigation#10767 within componentDidUpdate in Card.tsx.
- @react-navigation/stack@6.3.2
…navigation#10919) Test plan: Tested by rendering a dummy `View` for the header: ```js options={{ header: () => ( <View style={{ backgroundColor: 'rgba(0, 0, 0, 0.5)', height: 56, }} /> ), headerTransparent: true, }} ``` Fixes react-navigation#10822 Closes react-navigation#10902
- @react-navigation/native-stack@6.9.1
- @react-navigation/material-top-tabs@6.3.0
…entDidUpdate` (react-navigation#10871)" This reverts commit b9fb2d1.
- @react-navigation/stack@6.3.3
…eact-navigation#10972) Starting and building the web version of our Example app on Node v18 results in a build error described in webpack/webpack#14532 because of a breaking change in hashing algorithms used in newer Node.js versions (>=17). This issue got fixed in Webpack 5 but Webpack authors decided not to backport this fix to v4. Expo developer @byCedric suggests using this workaround: expo/expo-cli#4575 (comment) before Expo itself upgrades to Webpack 5. We have to pass that env variable to pretty much every command we use eg. before `build` script. Using `yarn lerna run prepack` directly still results in an error.
### Motivation This PR is a third attempt to migrate setNativeProps to state in @react-navigation/stack following the official Moving setNativeProps to state guide. After issues with the implementation of react-navigation#10767 and more problems with the fix in react-navigation#10871 we had to take a step back and rethink where the problem lays with these implementations. The solution was to move the state inside the CardSheet component and use useImperativeHandle to hoist a setter function up to Card to avoid triggering a rerender during Card animate function.
- flipper-plugin-react-navigation@1.3.15 - @react-navigation/stack@6.3.4
|
Hey mlecoq! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines. |
✅ Deploy Preview for react-navigation-example ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Hello, the PR got closed automatically due to some changes in the main branch. Would you be able to send a new PR? Thank you. |
|
Hey! This issue is closed and isn't watched by the core team. You are welcome to discuss the issue with others in this thread, but if you think this issue is still valid and needs to be tracked, please open a new issue with a repro. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
the new
tabBarGapoption helps to configuregapoption ofreact-native-tab-view: see https://github.com/satya164/react-native-tab-view#gapTest plan
Just set a tabBarGap option and check that there is a gap between tabs