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

headerRight randomly position the component incorrectly #432

Closed
lnmunhoz opened this issue Mar 21, 2020 · 84 comments · Fixed by #600, #613 or #2316
Closed

headerRight randomly position the component incorrectly #432

lnmunhoz opened this issue Mar 21, 2020 · 84 comments · Fixed by #600, #613 or #2316
Assignees
Labels
Area: Native Stack Bug Something isn't working NewArch Issues related only to new architecture OldArch Issues related only to old architecture

Comments

@lnmunhoz
Copy link

lnmunhoz commented Mar 21, 2020

I was testing the createNativeStackNavigation and ended up having this random issue where the component rendered by headerRight is being positioned incorrectly sometimes. There's nothing special in my code, I am just rendering a <Button> from react-native.

Here's the source code: https://github.com/lnmunhoz/react-native-experiments/blob/master/react-navigation-examples/examples/NativeNavigation.tsx.

Kapture 2020-03-21 at 20 56 50

Update

The issue also happens when the headerLargeTitle is false.

image

@WoLewicki
Copy link
Member

I cannot repro your issue either on a simulator or physical device. Upgrade packages to the newest version. Is the issue still present then? If so, please try to write which steps to take for the issue to occur.

@lnmunhoz
Copy link
Author

lnmunhoz commented Mar 25, 2020

Yes, I just tried again and the issue is still present. I did a fresh install and the issue persisted. Have you tried the repo I've sent?

The steps are simple:

  • Clone the repo
  • cd repo/react-navigation-examples
  • yarn && yarn start
  • And then open in iOS simulator

To create the stack I am using createNativeStackNavigator from react-native-screens/native-stack.

I created a snack to reproduce but also not being able to make it happen there:

The thing is, I am using a stack created by createNativeStackNavigator inside a stack from createStackNavigator.

I wanted to have parts of my app using the native stack for more simple screens where I don't need to manipulate the header too much, and others the js based stack so I can migrate incrementally.

Is that an issue or should work fine?

@WoLewicki
Copy link
Member

I cloned your project and also made a bare react-native project. The issue exists only while opening it through Expo Client. This means that the version of RNScreens in the current Expo Client is having the bug since it has RNScreens in version 2.0.0-alpha.12. Can you repro it that way to prove if it is right?

@WoLewicki
Copy link
Member

@lnmunhoz can I close it then? Is your issue solved?

@lnmunhoz
Copy link
Author

Let me try to reproduce one more time so we can figure out why it was happening.

I will update here in a couple hours.

@Deepp0925
Copy link

This is happens in bare RN project as well, so I doubt it has anything to do with Expo

@WoLewicki
Copy link
Member

@Deepp0925 does it happen with the newest RNScreens version: 2.4.0?

@Deepp0925
Copy link

Yes sir

@markmssd
Copy link

I have a workaround if it's a blocker for anyone here react-navigation/react-navigation#6746 (comment):

As a workaround I show the Icon after a few ms:

const [shouldRenderIcon, setShouldRenderIcon] = useState(false)

useEffect(() => {
const nbr = setTimeout(() => setShouldRenderIcon(true), 200)
return () => clearTimeout(nbr)
}, [])

return shouldRenderIcon ? <MyIcon /> : null

@lucasmotta
Copy link

I can also confirm that this bug still happens on [email protected] and [email protected].

@lucasmotta
Copy link

Hey @WoLewicki, do you want me to setup a sample project, showing this issue? Here's some screenshots demonstrating the problem:

This first one, is just adding a 44x44 Touchable button as the headerRight - you can see it's quite far to the left.
Screenshot 2020-04-30 at 17 45 37

If I inspect the element, you can see it's 44x44:
Screenshot 2020-04-30 at 17 51 30

If I add a marginRight: -20, it does "fix" the alignment, but not the issue itself.
Screenshot 2020-04-30 at 17 46 22

Now if I inspect the element, you can see it's only 24px wide, so almost half of the button is cropped and not easy to tap.
Screenshot 2020-04-30 at 17 46 44

It feels like this extra 20px on the right side added by the native wrapper is causing the problem - if that space would be removed entirely the problem would be solved. The back button works this way if you inspect it:

Screenshot 2020-04-30 at 17 54 17

@sbeigel
Copy link

sbeigel commented May 2, 2020

Using a custom compontent in headerCenter is affected by the same bug/problem...

@WoLewicki WoLewicki assigned WoLewicki and unassigned WoLewicki May 8, 2020
@WoLewicki
Copy link
Member

@osdnk

@FrankFundel
Copy link

same for me

@FrankFundel
Copy link

This happens almost everytime I start the app, this really should be fixed.

@WoLewicki
Copy link
Member

I believe it is the same issue as #322. Can you try the workaround submitted there and see if it works correctly?

@AlexSmirnov9107
Copy link

same for me

@SmirnovM91
Copy link

same for me too

@immortalx
Copy link

This is happening to me also with headerCenter and headerRight only in iOS, my custom component is wrongly positioned.
It only displays correctly when the default back button is present.

@zyofeng
Copy link

zyofeng commented Jun 25, 2020

Same problem Im having

const Stack = createNativeStackNavigator();
const AccountsScreen = () => {
return <Stack.Navigator screenOptions={(props) => ({
headerRight: () => <HeaderRight {...props} />,
})}>
<Stack.Screen name="Balances" component={Balances} />
...

interface Props {
navigation?: NavigationContainerRef;
}
export const HeaderRight = (props: Props) => {
const dispatch = useDispatch();
const {navigation} = props;
return <TouchableOpacity onPress={() => dispatch(lock())}>
<Text style={{color: 'red'}}>test

}

Sometimes the Text gets pushed off screen in ios, RN .62 with latest React Navigation.
Screen Shot 2020-06-25 at 4 44 38 PM

Screen Shot 2020-06-25 at 4 43 40 PM

@ortigozamatias
Copy link

ortigozamatias commented Jun 29, 2020

Same here using Expo SDK v38, which relies on ~2.9.0.

@FrankFundel
Copy link

+1

@trajano
Copy link

trajano commented Feb 14, 2024

@tboba I get it on the preview build on a real device when I build it on EAS at least on my app, but not on the above sample.
Your note about it being the first time is correct.

@fredrikburmester
Copy link

fredrikburmester commented Feb 29, 2024

I'm also getting this issue on Expo Router ~3.4.7 (SDK 50.0.0). It went away for me when moving more logic about the header into the layout instead of setting it on mount in the useEffect hook. Like this:

// app/(app)/_layout.tsx
<Stack.Screen
    name="settings"
    options={{
    ...
    title: "",
    headerLeft: () => (
        <View>
            ...
        </View>
        ),
    }}
/>
// app/(app)/settings.tsx
useEffect(() => {
    navigation.setOptions({
      headerRight: () => <HeaderRight onPress={() => saveMutation.mutate()} loading={saveMutation.isLoading} />,
    });
  }, [saveMutation]);

Might work for some of you!

@alduzy
Copy link
Member

alduzy commented Jun 18, 2024

I was unable to reproduce the issue with the latest version, tried with Expo Go, Expo prebuild, Bare RN. If the problem still persists with the latest version please attach a working repro.

@kkafar kkafar assigned kkafar and alduzy and unassigned kkafar Jun 28, 2024
alduzy added a commit that referenced this issue Jul 9, 2024
## Description

This PR gets rid of undesired white flashes during
`maybeAddToParentAndUpdateContainer`. The white flash was present on
paper architecture when `unmountOnBlur` option was set to true on parent
bottomStackNavigator (see repro).

The affected logic was previously introduced or changed by following
PRs:
- #600
- #613
- #643


The removed `_hasLayout` was initially added by
#600 in
order to resolve an issue:
#432.
However the logic was later changed by
#613 in
order to fix another issue and the added `_hasLayout` may not fix
anything eventually, as stated by [this
comment](#432 (comment)).

Fixes #1645.

## Changes

- removed `_hasLayout` variable
- added repros

## Screenshots / GIFs

### Before


https://github.com/software-mansion/react-native-screens/assets/91994767/226a32d7-728b-48dd-b21a-6a1e4195add2

### After


https://github.com/software-mansion/react-native-screens/assets/91994767/32febcf1-d159-4a9d-ae3a-373042732a6d

## Test code and steps to reproduce

- added `Test1645.js` repro to test examples
- added `Test432.tsx` repro to test examples

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <[email protected]>
alduzy added a commit that referenced this issue Jul 9, 2024
This PR gets rid of undesired white flashes during
`maybeAddToParentAndUpdateContainer`. The white flash was present on
paper architecture when `unmountOnBlur` option was set to true on parent
bottomStackNavigator (see repro).

The affected logic was previously introduced or changed by following
PRs:
- #600
- #613
- #643

The removed `_hasLayout` was initially added by
#600 in
order to resolve an issue:
#432.
However the logic was later changed by
#613 in
order to fix another issue and the added `_hasLayout` may not fix
anything eventually, as stated by [this
comment](#432 (comment)).

Fixes #1645.

- removed `_hasLayout` variable
- added repros

https://github.com/software-mansion/react-native-screens/assets/91994767/226a32d7-728b-48dd-b21a-6a1e4195add2

https://github.com/software-mansion/react-native-screens/assets/91994767/32febcf1-d159-4a9d-ae3a-373042732a6d

- added `Test1645.js` repro to test examples
- added `Test432.tsx` repro to test examples

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <[email protected]>
@alduzy alduzy added the Close when stale This issue is going to be closed when there is no activity for a while label Jul 17, 2024
@elliottkember
Copy link

elliottkember commented Aug 14, 2024

We're still getting this on v3.34.0 unfortunately. The behaviour has changed slightly – we're now getting this positioning in a react-navigation drawer header. It happens consistently now, rather than randomly.

We're also getting this strange "back" button positioning – this isn't a custom back button but rather a regular back button provided by react-navigation.

This only happens in a route defined at the root of the navigator, above the tab navigator. It's something to do with not specifying headerTitle in the header stack. I've been trying to make a repo that reproduces this behaviour, but haven't nailed down exactly what just yet.

image

It may have something to do with our usage of navigation.setOptions – we use this a little too liberally in our codebase to set navigation options as it's nice to have navigation options defined with the component's contents. We'll refactor that and see whether the behaviour improves.

We do use a stack navigator in a drawer – if I initialize that stack with an initial state, the header button is reliably in the wrong place. If I navigate to page 2 with a timeout, it renders in the right place. So something is up with the layout calculation on mount in a drawer. An edge case, but maybe it will help make this headerRight positioning more robust.

Still no repro repository, I'm afraid, but if we're able to tame this behaviour I'll update this comment.

@alduzy
Copy link
Member

alduzy commented Aug 19, 2024

@elliottkember Thanks for the message. Having a working example from you would be ideal.

@alduzy alduzy reopened this Aug 19, 2024
@thomasttvo
Copy link

thomasttvo commented Aug 20, 2024

One of the cases I can see is using setOptions to change the headerTitle and headerRight at the same time

useEffect(() => {
	navigation.setOptions({
      headerTitleAlign: 'center',
      title: x ? '123123' : '1',
      headerRight: x
        ? () => <HeaderRight />
        : undefined,
	})
}, [navigation, x])

When x changes from false to true, the headerRight is shown, but misaligned.
It also happens when you remove the back button. I guess basically anything that involves changing the layout while putting on a new header right.

If <HeaderRight> has an internal setTimeout that delays its rendering by a little maybe 100ms, then it's no longer misaligned.
This is v3.34.0

@github-actions github-actions bot removed the Close when stale This issue is going to be closed when there is no activity for a while label Aug 20, 2024
@thomasttvo
Copy link

thomasttvo commented Aug 22, 2024

I found out that the project that @elliottkember and I work on has a default headerRight: () => <View/> on the stack navigator's screen options. Removing that and the problem never comes back.

In fact the empty view wasn't the problem per se, it's the fact that the View expands while, at the same time, the title changes (or back button disappearing). In our case it was going from 0x0 to 20x20, but if you go from 20x20 to 30x20, the same issue happens

useTimeout(() => setX(true), 2000)
useEffect(() => {
	navigation.setOptions({
      headerTitleAlign: 'center',
	  headerBackVisible: !x,
      title: x ? '123123' : '1',
      headerRight: x
        ? () => <View style={{backgroundColor:'green', width: 20, height: 20}} />
        : () => <View style={{backgroundColor:'green', width: 1, height: 1}} />,
	})
}, [navigation, x])

@alduzy alduzy added OldArch Issues related only to old architecture NewArch Issues related only to new architecture labels Aug 23, 2024
@alduzy
Copy link
Member

alduzy commented Aug 23, 2024

@thomasttvo Thanks for the repro! I can confirm it's reproducible on both paper and fabric arch.

@js-ojha
Copy link

js-ojha commented Sep 10, 2024

Setting the headerTitle in InteractionManager worked for me.

useLayoutEffect(() => {
InteractionManager.runAfterInteractions(() => {
navigation.setOptions({
headerShown: true,
// eslint-disable-next-line react/no-unstable-nested-components
headerTitle: () => (
),
});
});
}, [dependency_A]);

@alduzy alduzy assigned kkafar and unassigned alduzy Sep 13, 2024
@alduzy alduzy closed this as completed in bc95fa4 Sep 27, 2024
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
…-mansion#2188)

## Description

This PR gets rid of undesired white flashes during
`maybeAddToParentAndUpdateContainer`. The white flash was present on
paper architecture when `unmountOnBlur` option was set to true on parent
bottomStackNavigator (see repro).

The affected logic was previously introduced or changed by following
PRs:
- software-mansion#600
- software-mansion#613
- software-mansion#643


The removed `_hasLayout` was initially added by
software-mansion#600 in
order to resolve an issue:
software-mansion#432.
However the logic was later changed by
software-mansion#613 in
order to fix another issue and the added `_hasLayout` may not fix
anything eventually, as stated by [this
comment](software-mansion#432 (comment)).

Fixes software-mansion#1645.

## Changes

- removed `_hasLayout` variable
- added repros

## Screenshots / GIFs

### Before


https://github.com/software-mansion/react-native-screens/assets/91994767/226a32d7-728b-48dd-b21a-6a1e4195add2

### After


https://github.com/software-mansion/react-native-screens/assets/91994767/32febcf1-d159-4a9d-ae3a-373042732a6d

## Test code and steps to reproduce

- added `Test1645.js` repro to test examples
- added `Test432.tsx` repro to test examples

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <[email protected]>
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
## Description

This PR fixes the incorrect position of the custom header items when
updating more than one option at the same time. The proposed solution
let us remove the previous fix for a similar problem:
software-mansion#2248 which
only fixed the issue on fabric.

Fixes software-mansion#432, software-mansion#2231.

## Changes

- forced re-layout of the navigation controller when subviews are
updated
- removed previous fix
- updated `Test432` repro

<!--

## Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

### Before

### After

-->

## Test code and steps to reproduce

- use `Test432` and `Test2231` to test this fix on both architectures.

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
kkafar pushed a commit that referenced this issue Oct 25, 2024
## Description

This PR fixes the incorrect position of the custom header items when
updating more than one option at the same time. The proposed solution
let us remove the previous fix for a similar problem:
#2248 which
only fixed the issue on fabric.

Fixes #432, #2231.

## Changes

- forced re-layout of the navigation controller when subviews are
updated
- removed previous fix
- updated `Test432` repro

<!--

## Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

### Before

### After

-->

## Test code and steps to reproduce

- use `Test432` and `Test2231` to test this fix on both architectures.

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes

(cherry picked from commit bc95fa4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Native Stack Bug Something isn't working NewArch Issues related only to new architecture OldArch Issues related only to old architecture
Projects
None yet