Revert back the full page scroll logic + fix infinite scroll updates #45
Conversation
WalkthroughThis pull request updates the project's package version from 0.1.0 to 0.1.1. It modifies the iframe detection logic in the main application component by removing the server environment check and applying a simplified try-catch block to add a "not-in-iframe" class when the window is at the top level. Additionally, global CSS rules and selectors related to iframe scenarios have been removed and updated to rely on class-based overflow management. A new constant for height difference threshold is introduced in the iframe communication logic. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pages/_app.js (1)
7-11: Consider adding error logging in catch block.The empty catch block silently suppresses errors, which could make debugging difficult in production.
try { if (window.self === window.top) { document.body.classList.add("not-in-iframe"); } - } catch (e) {} + } catch (e) { + console.warn('Failed to determine iframe status:', e); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)pages/_app.js(1 hunks)styles.css(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- package.json
- styles.css
🔇 Additional comments (2)
pages/_app.js (2)
1-4: LGTM! Component structure follows Next.js best practices.The component maintains a clean structure with proper imports and follows the standard Next.js _app.js patterns.
Also applies to: 14-20
6-12: Verify the scroll behavior in different contexts.Please ensure that:
- Scrolling works correctly when the page is loaded directly (not in iframe)
- Scrolling behavior is appropriate when loaded within an iframe
- The transition between states doesn't cause layout shifts
✅ Verification successful
🌐 Web query:
What are the best practices for handling page scrolling in iframes with Next.js?💡 Result:
When implementing iframes with controlled scrolling behavior in Next.js applications, developers should combine HTML attributes, CSS styling, and framework-specific optimizations while prioritizing security and performance:
Scrollbar Control & Overflow Management
- Use
scrolling="no"attribute combined withoverflow:hiddenCSS<iframe src="/widget" scrolling="no" style={{overflow:'hidden'}} />This hides both vertical/horizontal scrollbars[2][5][7]. For targeted control:
.no-scroll-iframe { overflow: hidden }Security Hardening
- Apply sandbox restrictions:
<iframe sandbox="allow-scripts allow-same-origin" />
- Configure CSP headers in
next.config.js:headers: [{ source: '/(.*)', headers: [ { key: 'Content-Security-Policy', value: "frame-src 'self' https://trusted-domains.com" } ] }][4][6]
Performance Optimization
- Implement lazy loading:
<iframe loading="lazy" />
- Use dynamic imports for heavy iframes:
const DynamicIframe = dynamic(() => import('@/components/Iframe'), { ssr: false, loading: () => <SkeletonLoader /> })[3][4]
Responsive Behavior
Create aspect-ratio containers:.iframe-container { position: relative; padding-top: 56.25%; /* 16:9 */ } .iframe-container iframe { position: absolute; top:0; left:0; width:100%; height:100%; }[4][7]
Client-Side Workarounds
For cases where SSR strips attributes:useEffect(() => { document.querySelectorAll('iframe').forEach(iframe => { iframe.setAttribute('scrolling', 'no') }) }, [])[2][5]
Cross-Origin Communication
Safe message handling:useEffect(() => { window.addEventListener('message', handleIframeMessage) return () => window.removeEventListener('message', handleIframeMessage) }, []) const handleIframeMessage = (e) => { if(e.origin !== 'https://trusted-source.com') return // Process message }[4][6]
For complex scroll interactions combining iframes with page animations, consider integrating scroll libraries like Lenis through wrapper components while maintaining isolated iframe scroll contexts[1][8]. Always test across browsers and provide fallbacks for motion-sensitive users[4][7].
Citations:
- 1: https://devdreaming.com/blogs/nextjs-smooth-scrolling-with-lenis-gsap
- 2: https://meta.discourse.org/t/iframe-css-code-disable-scrolling-bar/83254
- 3: https://blog.logrocket.com/best-practices-react-iframes/
- 4: https://caisy.io/blog/nextjs-iframe-implementation
- 5: https://community.canvaslms.com/t5/Archived-Questions/ARCHIVED-Scrollbars-in-iFrames-How-to-remove/td-p/455998
- 6: https://blog.logrocket.com/ultimate-guide-iframes/
- 7: https://www.dhiwise.com/post/how-to-make-iframe-scrolling-easy-for-your-users
- 8: https://www.youtube.com/watch?v=rqfGQfNc3Hk
- 9: https://gsap.com/community/forums/topic/35850-nextjs-gsap-horizontal-scroll-how-container-section-need-to-feet-exactly-to-his-child/
- 10: https://gsap.com/community/forums/topic/32468-how-to-make-a-horizontal-scroll-using-nextjs/
Scroll Behavior Verification: Confirming Context-Sensitive Styling
The changes in
pages/_app.jsnow add the"not-in-iframe"class when the page is loaded directly, which aligns with the PR objective of reverting full-page scroll logic and removing redundant styles. Based on current best practices for handling iframes in Next.js, please ensure that:
- When loaded directly, the presence of
"not-in-iframe"triggers the intended scroll behavior.- In an iframe, the absence of
"not-in-iframe"prevents the application of full page scroll styles.- Transitioning between these states does not lead to any layout shifts.
Manual testing in both contexts is recommended to confirm that scrolling and layout behave as expected.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/iframeCommunication.js (1)
43-58: Consider implementing structured debugging and monitoring.Given the complexity of iframe height management and the need for debugging, consider implementing:
- A proper logging/monitoring strategy using a logging library
- Performance metrics collection for height changes
- Error boundaries to catch and report issues
This will provide better insights into production issues without relying on console logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/iframeCommunication.js(2 hunks)
🔇 Additional comments (2)
components/iframeCommunication.js (2)
4-4: LGTM! Good improvement in code maintainability.Extracting the magic number into a named constant improves code readability and maintainability.
48-48: LGTM! Clean usage of the new constant.The threshold check is now more maintainable using the named constant.
bobbykolev
changed the title
Summary by CodeRabbit
Chores
Refactor
Style
Enhancements