-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Enable scroll restoration #148
base: main
Are you sure you want to change the base?
Conversation
This PR enables scroll restoration for the app, using next.js's experimental `scrollRestoration` feature, which overrides the default browser behavior of restoring the scroll position to the last saved position. The scenario is this: 1. User navigates to a page with a scrollable content. 2. User scrolls down to the bottom of the page. 3. User navigates to another page. 4. User navigates back to the previous page. 5. Result - Expected behavior: The scroll position is restored to the last saved position, which is the bottom of the page. - Actual behavior: The scroll position is restored to the top of the page.
WalkthroughThe changes in this pull request involve modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
next.config.js (1)
10-57
: LGTM: Well-structured remote patterns with secure configurationThe remote patterns are well-organized and properly secured with HTTPS. However, the empty port specification is redundant for HTTPS URLs.
Consider removing the empty port specifications to reduce config verbosity:
{ protocol: 'https', hostname: 'image.tmdb.org', - port: '', pathname: '/**', },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
next.config.js
(1 hunks)
🔇 Additional comments (1)
next.config.js (1)
3-5
: Verify experimental feature stability
While the implementation correctly enables scroll restoration, please note that this is using an experimental Next.js feature which may have stability implications or break in future versions.
Let's check the Next.js version and documentation:
Consider:
- Adding a comment explaining why this experimental feature is needed
- Creating a tracking issue to monitor the feature's stability status in future Next.js releases
✅ Verification successful
Experimental scroll restoration feature is appropriate for Next.js 13.4
The scroll restoration feature is a stable experimental feature in Next.js 13.4.x, and its usage here is appropriate. The implementation is clean and follows the official configuration pattern. Since this is the only experimental feature enabled and it addresses a specific UX need, the potential stability risks are contained.
- The feature is enabled in a standard way through the
experimental
config object - Using Next.js version 13.4.19 where this feature is well-documented
- No other experimental features are enabled, minimizing potential stability issues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Next.js version and whether this feature is still experimental
cat package.json | jq '.dependencies.next, .devDependencies.next'
# Look for any existing experimental features
rg -l "experimental.*scrollRestoration"
Length of output: 125
Script:
#!/bin/bash
# Let's check if there are any existing experimental features in next.config.js
cat next.config.js
# Let's also check if there are any documented issues or discussions about this feature
rg -i "scroll.*restoration" README.md CHANGELOG.md
Length of output: 1672
This PR enables scroll restoration for the app, using next.js's experimental
scrollRestoration
feature, which overrides the default browser behavior of restoring the scroll position to the last saved position.The scenario is this:
This is my first contribution to this repo, let me know if I'm missing anything!
Thanks for your great work.
Summary by CodeRabbit