feat: Enhance UI components with improved styling and functionality - #572
Conversation
- Updated AddRefundAccountModal to include dynamic error styling for account number input. - Added PlayPromoButton to AppLayout for better user engagement on the homepage. - Refined Navbar dropdown behavior with improved positioning and event handling. - Enhanced PlayPromoModal with better dismiss functionality and local storage management for banner visibility. - Updated PlayShell component to clarify navigation back to the homepage.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughChanges update refund input error styling, portaled navbar dropdown behavior, Play promo modal/banner persistence and layout, and homepage/header wiring for Play promo entry points. ChangesUI and interaction updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/components/AppLayout.tsx (1)
56-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
mb-16andmd:mb-[64px]are the same value — the responsive override is a no-op.Tailwind's default spacing scale makes
mb-16equal to64px, somd:mb-[64px]never changes anything from the basemb-16. If distinct mobile/desktop offsets were intended (banner heights differ slightly: 65px mobile vs 64px desktop, both plusmt-1), this doesn't achieve that.🔧 Example if distinct spacing was intended
className={`relative ${showPlayPromoBanner - ? "mb-16 md:mb-[64px]" + ? "mb-[69px] md:mb-[68px]" : config.maintenanceEnabled ? "mb-16" : "" }`}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/AppLayout.tsx` around lines 56 - 61, The `showPlayPromoBanner` branch in `AppLayout` uses `mb-16 md:mb-[64px]`, but those values are identical so the `md` override has no effect. Update the `className` logic in `AppLayout` to use a truly different desktop offset if the banner spacing is meant to change across breakpoints, keeping the conditional structure around `showPlayPromoBanner` and `config.maintenanceEnabled` intact.app/components/AddRefundAccountModal.tsx (1)
276-296: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWire
aria-invalid/aria-describedbyto the visual error state.The border now reflects
accountNumberErrorvisually, but the input isn't programmatically associated with the error for assistive tech. Screen reader users won't hear the validation message when returning to fix the field.♿️ Suggested fix
<input id="refund-account-number" type="text" inputMode="numeric" autoComplete="off" value={accountNumber} onChange={(e) => { setAccountNumber(e.target.value); setFormError(null); }} placeholder={getOfframpAccountIdentifierPlaceholder(currency, selectedInstitution?.type)} + aria-invalid={!!accountNumberError} + aria-describedby={accountNumberError ? "refund-account-number-error" : undefined} className={classNames( "w-full rounded-xl border bg-white px-3.5 py-3 text-sm text-neutral-900 outline-none transition-colors placeholder:text-neutral-400 focus:border-blue-500 focus:ring-2 focus:ring-blue-500/25 dark:bg-[`#202020`] dark:text-white dark:placeholder:text-white/40 dark:focus:border-blue-500 dark:focus:ring-blue-500/35", accountNumberError ? "border-red-500 dark:border-red-500" : "border-neutral-200 dark:border-white/[0.12]", )} /> {accountNumberError ? ( - <InputError message={accountNumberError} /> + <InputError id="refund-account-number-error" message={accountNumberError} /> ) : null}(
InputErrorwould need to forward anidprop; not shown in this diff.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/AddRefundAccountModal.tsx` around lines 276 - 296, The refund account number field in AddRefundAccountModal shows a visual error state via accountNumberError, but it is not linked to the validation message for assistive technologies. Update the input’s accessibility props so the refund-account-number field sets aria-invalid when accountNumberError is present and points aria-describedby to the rendered InputError message. To do that, make InputError support a stable id and connect it from the input and the existing accountNumberError conditional rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/Navbar.tsx`:
- Around line 161-173: The logo dropdown only closes via hover/trigger
interaction, so touch users can’t dismiss it by tapping outside. Add an
outside-press fallback in Navbar.tsx by wiring a pointerdown or mousedown
document listener alongside closeLogoDropdownUnlessHovered and isDropdownOpen,
and have it call setIsDropdownOpen(false) when the event target is outside both
dropdownRef and portaledDropdownRef. Use the existing refs and the
closeLogoDropdownUnlessHovered logic to keep the behavior consistent.
In `@app/components/PlayPromo.tsx`:
- Around line 120-135: usePlayPromoBannerVisible currently hardcodes visible to
true, which causes returning users to briefly see the promo banner until the
effect syncs with storage. Update the hook to initialize state from
isBannerDismissed() using a lazy useState initializer, and keep the existing
sync effect/event listeners in place so PlayPromoButton and other consumers read
the correct initial visibility from the start.
- Line 187: The close animation in PlayPromo’s Dialog is not being played
because the dialog unmounts immediately when isOpen turns false, so the exit
state never has a chance to run. Update the Dialog rendering block by wrapping
it in AnimatePresence and marking the Dialog as static so it stays mounted
through the animation, or remove the exit variant from the motion element if you
do not want an unmount animation. Use the PlayPromo component’s Dialog and
AnimatePresence/motion setup to apply the fix.
---
Nitpick comments:
In `@app/components/AddRefundAccountModal.tsx`:
- Around line 276-296: The refund account number field in AddRefundAccountModal
shows a visual error state via accountNumberError, but it is not linked to the
validation message for assistive technologies. Update the input’s accessibility
props so the refund-account-number field sets aria-invalid when
accountNumberError is present and points aria-describedby to the rendered
InputError message. To do that, make InputError support a stable id and connect
it from the input and the existing accountNumberError conditional rendering.
In `@app/components/AppLayout.tsx`:
- Around line 56-61: The `showPlayPromoBanner` branch in `AppLayout` uses `mb-16
md:mb-[64px]`, but those values are identical so the `md` override has no
effect. Update the `className` logic in `AppLayout` to use a truly different
desktop offset if the banner spacing is meant to change across breakpoints,
keeping the conditional structure around `showPlayPromoBanner` and
`config.maintenanceEnabled` intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56a63fd7-5fd8-4c69-b0dc-400346ebeebc
📒 Files selected for processing (5)
app/components/AddRefundAccountModal.tsxapp/components/AppLayout.tsxapp/components/Navbar.tsxapp/components/PlayPromo.tsxapp/components/play/PlayShell.tsx
…ience - Added event handling in Navbar to close dropdown when clicking outside. - Updated PlayPromoModal to utilize AnimatePresence for smoother animations and improved dismiss functionality. - Enhanced visibility management for the PlayPromo banner with local storage integration.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/PlayPromo.tsx (1)
120-140: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStart
readyasfalseon the first render.
readyis derived fromtypeof window !== "undefined", so the server returnsfalsewhile the client's first hydration render returnstrue.AppLayoutuses this hook to decide whether to renderPlayPromoBanner/PlayPromoButton, which can make the server and client trees diverge and bring back the banner flash this gate is meant to prevent. Initializereadytofalseon both passes and flip it inuseEffect.🔧 Fix
- const [ready, setReady] = useState(() => typeof window !== "undefined"); + const [ready, setReady] = useState(false);The same pattern in
PlayPromoBannershould use the same initializer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/PlayPromo.tsx` around lines 120 - 140, `usePlayPromoBannerVisible` is initializing `ready` differently on server and client, which can cause hydration mismatch and banner flash; start `ready` as false on the first render and set it to true inside the existing `useEffect` after syncing visibility. Make the same initializer change in `PlayPromoBanner` wherever it uses the same ready-gating pattern, so both components use a consistent client-only flip after mount.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/components/PlayPromo.tsx`:
- Around line 120-140: `usePlayPromoBannerVisible` is initializing `ready`
differently on server and client, which can cause hydration mismatch and banner
flash; start `ready` as false on the first render and set it to true inside the
existing `useEffect` after syncing visibility. Make the same initializer change
in `PlayPromoBanner` wherever it uses the same ready-gating pattern, so both
components use a consistent client-only flip after mount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84b26246-11f7-47a5-a514-f4dae6dcfca8
📒 Files selected for processing (2)
app/components/Navbar.tsxapp/components/PlayPromo.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/Navbar.tsx
Description
This PR polishes the Noblocks Play homepage promo experience and fixes several UI layering and form-feedback issues introduced while aligning the banner/modal with Figma.
Noblocks Play promo (homepage)
PlayPromoButtontoAppLayoutso a compact Play CTA remains in the navbar area after the promo banner is dismissed.PlayPromoBannerlayout for mobile and desktop:max-w-screen-2xlcontainer.PlayPromoModaldismiss behavior and persistence:localStorage(noblocks_play_promo_dismissed) so it only shows once per visitor.noblocks_play_promo_banner_dismissed) and drivesusePlayPromoBannerVisible()for layout spacing andPlayPromoButtonvisibility.pointer-events-nonewith interactive content onpointer-events-auto.z-[30]).z-[55]) so the cup tip and heads can pop above the strip.document.bodyatz-[60]so it remains visible above the collage.Navbar
Refund account modal
AddRefundAccountModalto show dynamic error styling on the account number input (border-red-500) and inlineInputErrorwhen validation fails.Play experience
PlayShellcopy/labels so navigation back to swap is clearer (Home on mobile, Back to homepage on desktop).Impacts
Alternatives considered
References
No linked issues for this PR.
Testing
Manual verification on homepage with
fantasyEnabledenabled:PlayPromoButtonappears, spacing updates correctly.AddRefundAccountModal: invalid account number shows red border + error message./play: Back to homepage / Home link returns to/.Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
If you want this committed or opened as a PR via
gh, switch to Agent mode and I can do that for you.Summary by CodeRabbit
New Features
Bug Fixes