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

Fix useScrollToTop not working when nesting multiple tab navigators #11063

Conversation

anthonyguay
Copy link
Contributor

@anthonyguay anthonyguay commented Dec 1, 2022

Motivation

This PR answers this open issue: #11045

When tapping on a bottom tab, the screen scrolls back to the top. However, when adding a second tab navigator at the top of the screen, tapping the bottom tap does not automatically scroll to top anymore. Only tapping the top navigation tab will scroll the screen up. It is expected that the bottom tab tap will still scroll the screen to the top, just like tapping the status bar.

Context

This issue has previously been reported here by @iirovi, and a pull request to fix the issue has been proposed by @Gregoirevda here. Recent changes on main automatically closed the pull request, so I'm posting his solution here again at @satya164's request.

We tested this solution on our app and it works perfectly well. Previously, tapping the bottom tab would not scroll back to the top when a top tab bar was present, but this fixes the issue. You can find the package versions in the open issue.

Behavior example

Twitter has both top and bottom navigation bars. When tapping on the bottom tap, the screen scrolls back to the top. Tapping the top tab also scroll to the top.

Untitled.mp4

@github-actions
Copy link

github-actions bot commented Dec 1, 2022

Hey anthonyguay! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines.

@netlify
Copy link

netlify bot commented Dec 1, 2022

Deploy Preview for react-navigation-example ready!

Name Link
🔨 Latest commit b338718
🔍 Latest deploy log https://app.netlify.com/sites/react-navigation-example/deploys/63907ac8281f910009a8bada
😎 Deploy Preview https://deploy-preview-11063--react-navigation-example.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Dec 4, 2022

Hey autofix-ci[bot]! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines.

packages/native/src/useScrollToTop.tsx Outdated Show resolved Hide resolved
packages/native/src/useScrollToTop.tsx Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2022

Codecov Report

Base: 74.29% // Head: 74.17% // Decreases project coverage by -0.11% ⚠️

Coverage data is based on head (a570ed0) compared to base (b3722fd).
Patch coverage: 0.00% of modified lines in pull request are covered.

❗ Current head a570ed0 differs from pull request most recent head bd76a96. Consider uploading reports for the commit bd76a96 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11063      +/-   ##
==========================================
- Coverage   74.29%   74.17%   -0.12%     
==========================================
  Files         176      176              
  Lines        5578     5587       +9     
  Branches     2182     2186       +4     
==========================================
  Hits         4144     4144              
- Misses       1385     1394       +9     
  Partials       49       49              
Impacted Files Coverage Δ
packages/native/src/useScrollToTop.tsx 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Looks like TS is failing. Can you fix the errors?

packages/native/src/useScrollToTop.tsx Outdated Show resolved Hide resolved
@satya164 satya164 merged commit 7e38194 into react-navigation:main Dec 7, 2022
@Gregoirevda
Copy link

@anthonyguay thanks for getting this is main. We'll finally be able to remove our patch 👍

@github-actions
Copy link

github-actions bot commented Dec 7, 2022

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.

satya164 added a commit that referenced this pull request Dec 7, 2022
…11063)

**Motivation**

This PR answers this open issue:
#11045

When tapping on a bottom tab, the screen scrolls back to the top.
However, when adding a second tab navigator at the top of the screen,
tapping the bottom tap does not automatically scroll to top anymore.
Only tapping the top navigation tab will scroll the screen up. It is
expected that the bottom tab tap will still scroll the screen to the
top, just like tapping the status bar.

**Context**

This issue has previously been reported
[here](#8586)
by [@iirovi](https://github.com/iirovi), and a pull request to fix the
issue has been proposed by
[@Gregoirevda](https://github.com/Gregoirevda)
[here](#9434).
Recent changes on main automatically closed the pull request, so I'm
posting his solution here again at @satya164's
[request](#9434 (comment)).

We tested this solution on our app and it works perfectly well.
Previously, tapping the bottom tab would not scroll back to the top when
a top tab bar was present, but this fixes the issue. You can find the
package versions in [the open
issue](#11045).


**Behavior example**

Twitter has both top and bottom navigation bars. When tapping on the
bottom tap, the screen scrolls back to the top. Tapping the top tab also
scroll to the top.

<img
src="https://user-images.githubusercontent.com/4307396/205150557-64787dfc-ed77-4a2f-88f3-205b05b6aead.mp4"
width="300">

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Satyajit Sahoo <satyajit.happy@gmail.com>
satya164 added a commit that referenced this pull request Dec 7, 2022
…11063)

**Motivation**

This PR answers this open issue:
#11045

When tapping on a bottom tab, the screen scrolls back to the top.
However, when adding a second tab navigator at the top of the screen,
tapping the bottom tap does not automatically scroll to top anymore.
Only tapping the top navigation tab will scroll the screen up. It is
expected that the bottom tab tap will still scroll the screen to the
top, just like tapping the status bar.

**Context**

This issue has previously been reported
[here](#8586)
by [@iirovi](https://github.com/iirovi), and a pull request to fix the
issue has been proposed by
[@Gregoirevda](https://github.com/Gregoirevda)
[here](#9434).
Recent changes on main automatically closed the pull request, so I'm
posting his solution here again at @satya164's
[request](#9434 (comment)).

We tested this solution on our app and it works perfectly well.
Previously, tapping the bottom tab would not scroll back to the top when
a top tab bar was present, but this fixes the issue. You can find the
package versions in [the open
issue](#11045).


**Behavior example**

Twitter has both top and bottom navigation bars. When tapping on the
bottom tap, the screen scrolls back to the top. Tapping the top tab also
scroll to the top.

<img
src="https://user-images.githubusercontent.com/4307396/205150557-64787dfc-ed77-4a2f-88f3-205b05b6aead.mp4"
width="300">

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Satyajit Sahoo <satyajit.happy@gmail.com>
@anthonyguay
Copy link
Contributor Author

@anthonyguay thanks for getting this is main. We'll finally be able to remove our patch 👍

All thanks to you @Gregoirevda ! We're also going to be able to remove the patch 👍

satya164 added a commit that referenced this pull request Dec 11, 2022
…11063)

**Motivation**

This PR answers this open issue:
#11045

When tapping on a bottom tab, the screen scrolls back to the top.
However, when adding a second tab navigator at the top of the screen,
tapping the bottom tap does not automatically scroll to top anymore.
Only tapping the top navigation tab will scroll the screen up. It is
expected that the bottom tab tap will still scroll the screen to the
top, just like tapping the status bar.

**Context**

This issue has previously been reported
[here](#8586)
by [@iirovi](https://github.com/iirovi), and a pull request to fix the
issue has been proposed by
[@Gregoirevda](https://github.com/Gregoirevda)
[here](#9434).
Recent changes on main automatically closed the pull request, so I'm
posting his solution here again at @satya164's
[request](#9434 (comment)).

We tested this solution on our app and it works perfectly well.
Previously, tapping the bottom tab would not scroll back to the top when
a top tab bar was present, but this fixes the issue. You can find the
package versions in [the open
issue](#11045).


**Behavior example**

Twitter has both top and bottom navigation bars. When tapping on the
bottom tap, the screen scrolls back to the top. Tapping the top tab also
scroll to the top.

<img
src="https://user-images.githubusercontent.com/4307396/205150557-64787dfc-ed77-4a2f-88f3-205b05b6aead.mp4"
width="300">

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Satyajit Sahoo <satyajit.happy@gmail.com>
satya164 added a commit that referenced this pull request Feb 17, 2023
…11063)

**Motivation**

This PR answers this open issue:
#11045

When tapping on a bottom tab, the screen scrolls back to the top.
However, when adding a second tab navigator at the top of the screen,
tapping the bottom tap does not automatically scroll to top anymore.
Only tapping the top navigation tab will scroll the screen up. It is
expected that the bottom tab tap will still scroll the screen to the
top, just like tapping the status bar.

**Context**

This issue has previously been reported
[here](#8586)
by [@iirovi](https://github.com/iirovi), and a pull request to fix the
issue has been proposed by
[@Gregoirevda](https://github.com/Gregoirevda)
[here](#9434).
Recent changes on main automatically closed the pull request, so I'm
posting his solution here again at @satya164's
[request](#9434 (comment)).

We tested this solution on our app and it works perfectly well.
Previously, tapping the bottom tab would not scroll back to the top when
a top tab bar was present, but this fixes the issue. You can find the
package versions in [the open
issue](#11045).


**Behavior example**

Twitter has both top and bottom navigation bars. When tapping on the
bottom tap, the screen scrolls back to the top. Tapping the top tab also
scroll to the top.

<img
src="https://user-images.githubusercontent.com/4307396/205150557-64787dfc-ed77-4a2f-88f3-205b05b6aead.mp4"
width="300">

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Satyajit Sahoo <satyajit.happy@gmail.com>
satya164 added a commit that referenced this pull request Feb 17, 2023
…11063)

**Motivation**

This PR answers this open issue:
#11045

When tapping on a bottom tab, the screen scrolls back to the top.
However, when adding a second tab navigator at the top of the screen,
tapping the bottom tap does not automatically scroll to top anymore.
Only tapping the top navigation tab will scroll the screen up. It is
expected that the bottom tab tap will still scroll the screen to the
top, just like tapping the status bar.

**Context**

This issue has previously been reported
[here](#8586)
by [@iirovi](https://github.com/iirovi), and a pull request to fix the
issue has been proposed by
[@Gregoirevda](https://github.com/Gregoirevda)
[here](#9434).
Recent changes on main automatically closed the pull request, so I'm
posting his solution here again at @satya164's
[request](#9434 (comment)).

We tested this solution on our app and it works perfectly well.
Previously, tapping the bottom tab would not scroll back to the top when
a top tab bar was present, but this fixes the issue. You can find the
package versions in [the open
issue](#11045).


**Behavior example**

Twitter has both top and bottom navigation bars. When tapping on the
bottom tap, the screen scrolls back to the top. Tapping the top tab also
scroll to the top.

<img
src="https://user-images.githubusercontent.com/4307396/205150557-64787dfc-ed77-4a2f-88f3-205b05b6aead.mp4"
width="300">

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Satyajit Sahoo <satyajit.happy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants