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.

Sign up for GitHub

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

Jump to bottom

Wrong layout when updating headerLeft and headerRight on new arch #2231

Closed
janicduplessis opened this issue Jul 5, 2024 · 1 comment · Fixed by #2248
Closed

Wrong layout when updating headerLeft and headerRight on new arch #2231

janicduplessis opened this issue Jul 5, 2024 · 1 comment · Fixed by #2248
Assignees
@alduzy
Labels
NewArch Issues related only to new architecture Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided

Comments

@janicduplessis
Copy link
Contributor

janicduplessis commented Jul 5, 2024 •
edited
Loading

Description

When the headerLeft is removed and headerRight has updated layout at the same time in native-stack, it results in the headerRight having incorrect layout.

Workaround:

If I update them in separate ticks (using setTimeout) layout is correct.

Steps to reproduce

Using this code in FabricExample app HeaderOptions.tsx

  • Go to "Header Options" example
  • Click the "Header large item" button
  • See that the header right item green square is off screen
  • Comment out line 59 where we set headerLeft
  • Click the "Header large item" button
  • See that the header right item green square has now correct layout

Updating both headerLeft and headerRight (bug):

image image

Updating only headerRight (correct)

image image

Snack or a link to a repository

https://github.com/janicduplessis/react-native-screens/tree/%40janic/header-bug-1

Screens version

main@e5a6220

React Native version

0.74

Platforms

iOS

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Fabric (New Architecture)

Build type

Debug mode

Device

iOS simulator

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 Jul 5, 2024
@kkafar kkafar added the NewArch Issues related only to new architecture label Jul 8, 2024
@alduzy alduzy self-assigned this Jul 15, 2024
@alduzy
Copy link
Member

alduzy commented Jul 15, 2024 •
edited
Loading

Hi! Thanks for reporting. I'm going to take a look at it.
Wrapping the right item in a useCallback hook should do the trick until the problem's solved.
Edit: I misunderstood the issue initially, but I can see the problem now.

@alduzy alduzy mentioned this issue Jul 16, 2024
Merged
1 task
alduzy added a commit that referenced this issue Jul 19, 2024
@alduzy
fix(iOS): header left and right layout on fabric (#2248)
5cd3b11 
## Description

This PR fixes the header layout mismatch when updating both `headerLeft`
and `headerRight` options simultaneously using `navigation.setOptions`.

Fixes #2231 .

## Changes

- Forced subview to re-layout when updating layoutMetrics
- added `Test2231.tsx` repro


## Screenshots / GIFs

### Before
![Screenshot 2024-07-16 at 12 05
14](https://github.com/user-attachments/assets/37a5a77d-1cd5-457f-901e-9dd5b4065a8f)

### After
![Screenshot 2024-07-16 at 12 04
49](https://github.com/user-attachments/assets/025db807-ef6d-46f2-b4c0-cd9f06e03d93)

## Test code and steps to reproduce

- Use `Test2231.tsx` repro

## Checklist

- [x] Ensured that CI passes
@alduzy alduzy mentioned this issue Aug 23, 2024
Merged
2 tasks
alduzy added a commit that referenced this issue Sep 27, 2024
@alduzy
fix(iOS): right header incorrect position (#2316)
bc95fa4 
## 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
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
@alduzy @ja1ns
fix(iOS): header left and right layout on fabric (software-mansion#2248)
4546d0c 
## Description

This PR fixes the header layout mismatch when updating both `headerLeft`
and `headerRight` options simultaneously using `navigation.setOptions`.

Fixes software-mansion#2231 .

## Changes

- Forced subview to re-layout when updating layoutMetrics
- added `Test2231.tsx` repro


## Screenshots / GIFs

### Before
![Screenshot 2024-07-16 at 12 05
14](https://github.com/user-attachments/assets/37a5a77d-1cd5-457f-901e-9dd5b4065a8f)

### After
![Screenshot 2024-07-16 at 12 04
49](https://github.com/user-attachments/assets/025db807-ef6d-46f2-b4c0-cd9f06e03d93)

## Test code and steps to reproduce

- Use `Test2231.tsx` repro

## Checklist

- [x] Ensured that CI passes
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
@alduzy @ja1ns
fix(iOS): right header incorrect position (software-mansion#2316)
1a83f0c 
## 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
@alduzy @kkafar
fix(iOS): right header incorrect position (#2316)
29d3c2c 
## 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
Assignees

@alduzy alduzy

Labels
NewArch Issues related only to new architecture Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(iOS): header left and right layout on fabric software-mansion/react-native-screens
3 participants
@janicduplessis @kkafar @alduzy