fix: mobile drawer close-on-outside-click, navbar layout offset, testimonials error handling & leaderboard UI#1670
Conversation
✅ Deploy Preview for reactplayio ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey! contributor, thank you for opening a Pull Request 🎉.
@reactplay/maintainers will review your submission soon and give you helpful feedback.
If you're interested in continuing your contributions to open source and want to be a part of a welcoming and fantastic community, we invite you to join our ReactPlay Discord Community.
Show your support by starring ⭐ this repository. Thank you and we appreciate your contribution to open source!
Stale Marking : After 30 days of inactivity this issue/PR will be marked as stale issue/PR and it will be closed and locked in 7 days if no further activity occurs.
There was a problem hiding this comment.
Pull request overview
Fixes mobile swipe interactions for the homepage Testimonials carousel by adjusting Swiper touch handling and nested scroll behavior. The PR also introduces a new “BMR & TDEE Calculator” play, which is unrelated to the stated carousel bugfix scope.
Changes:
- Update Testimonials carousel configuration to better capture touch gestures on mobile.
- Update testimonial quote container to allow vertical scrolling without blocking horizontal swipes.
- Add a new “BMR & TDEE Calculator” play (component + styles + assets + README) and export it via
src/plays/index.js.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/common/Testimonial/TestimonialSection.jsx | Adds Swiper props (grabCursor, touchEventsTarget="container") to improve mobile swipe handling. |
| src/common/Testimonial/TestimonialCard.jsx | Adds touchAction: 'pan-y' on the scrollable quote area to prevent it from capturing horizontal swipes. |
| src/plays/index.js | Adds export for the new BMR/TDEE play (also broadens PR scope beyond the described bugfix). |
| src/plays/bmr-tdee-calculator/BmrTdeeCalculator.jsx | New BMR/TDEE calculator play implementation. |
| src/plays/bmr-tdee-calculator/styles.css | Styles for the new calculator play. |
| src/plays/bmr-tdee-calculator/cover.svg | Cover asset for the new calculator play. |
| src/plays/bmr-tdee-calculator/Readme.md | Documentation for the new calculator play (contains a formula inconsistency noted in comments). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Scroll.Manually.webm✅ Auto-play carousel |
|
@aniketmishra-0 there are some reviews by copilot can you please check? |
|
Ofcourse
|
…y#1667) - Add touchEventsTarget='container' to Swiper so touch listeners attach to container instead of wrapper - Add grabCursor for visual drag feedback on desktop - Fix className bug: home && 'h-32' → home ? 'h-32' : '' to prevent 'false' string in DOM - Add touchAction: 'pan-y' on blockquote to allow horizontal swipes to pass through to Swiper Closes reactplay#1667
aniketmishra-0
force-pushed
the
fix/testimonials-carousel-swipe-navigation
branch
from
0dd66f7 to
f031618
Compare
|
@priyankarpal check |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rboard UI - Fix mobile drawer not closing on outside click (useRef + click-outside listener) - Fix CSS transform creating containing block issue (translateY(0) → none) - Add global padding-top for fixed navbar offset (64px, excluding footer) - Add extra home hero padding for activity banner - Add try/catch error handling in testimonial fetches - Fix ESLint jsx-sort-props (shorthand booleans first) - Center leaderboard loading spinner - Remove overflow-hidden from leaderboard page - Import leaderBoard.css in LeaderBoard component
aniketmishra-0
changed the title
Could you please review the changed code? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/common/playleaderboard/LeaderBoard.jsx:78
topContributorOfTheMonthis initialized as[], which is truthy. As soon aspublishedPlaysloads (before the monthly contributor fetch resolves), the render path will mountTopPlayCreatorOfTheMonthwith an array, and that component immediately dereferencestopPlayCreatorOfTheMonth.avatarUrl/displayName, causing a runtime crash. InitializetopContributorOfTheMonthtonull/undefined(or{}) and tighten the render condition to only render when the expected fields are present (e.g., checktopContributorOfTheMonth?.avatarUrl).
{publishedPlays.length && (topContributorOfTheMonth || top10Contributors) ? (
<div className=" overflow-auto lg:flex flex-row justify-center">
{topContributorOfTheMonth && (
<TopPlayCreatorOfTheMonth topPlayCreatorOfTheMonth={topContributorOfTheMonth} />
)}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
2 participants
Summary
This PR fixes multiple UI/UX issues across the mobile navigation drawer, fixed navbar layout, testimonials section, and leaderboard page.
Changes
1. Mobile Drawer — Close on Outside Click
Files:
src/common/header/HeaderNav.jsxuseRefon the menu panel (<ul>) and auseEffectthat listens formousedown/touchstartevents ondocumentuseCallbackimport2. CSS
transformContaining Block FixFile:
src/common/header/header.css.nav--visiblefromtransform: translateY(0)totransform: nonetranslateY(0)creates a new CSS containing block, which constrainsposition: fixeddescendants (the drawer overlay) to the 64px header height instead of the full viewport3. Fixed Navbar — Global Content Offset
File:
src/common/header/header.css.nav-wrapper ~ *:not(footer) { padding-top: 64px; }to push all page content below the fixed navbar:not(footer)ensures the footer doesn't get unwanted top padding4. Home Hero — Extra Padding for Activity Banner
File:
src/common/home/home.csspadding-top: 32pxto.app-home-bodyfor the activity banner offset (on top of the global 64px)5. Testimonials — Error Handling
Files:
src/common/Testimonial/TestimonialSection.jsx,src/common/Testimonial/Testimonials.jsxsubmit()calls intry/catchblocks6. ESLint
react/jsx-sort-propsFixFile:
src/common/Testimonial/TestimonialSection.jsxgrabCursor,rewind) before all other props on the<Swiper>component per ESLint rule7. Leaderboard — Centered Loading Spinner
Files:
src/common/playleaderboard/LeaderBoard.jsx,src/common/playleaderboard/leaderBoard.cssimport './leaderBoard.css'(was missing).leaderboard-loaderstyles: centered flex container withmin-height: calc(100vh - 160px)app-body-overflow-hiddenclass from<main>to fix excessive bottom whitespaceFiles Changed (8)
src/common/header/HeaderNav.jsxsrc/common/header/header.csssrc/common/home/home.csssrc/common/Testimonial/TestimonialSection.jsxsrc/common/Testimonial/Testimonials.jsxsrc/common/playleaderboard/LeaderBoard.jsxsrc/common/playleaderboard/leaderBoard.csspackage.json