feat(web): preserve scroll position and add query params to about and brand pages #2100
Conversation
… brand pages - Add resetScroll: false to navigate calls in about.tsx to preserve scroll position - Add query params handling to brand.tsx (type: visual/typography/color, id) - Add resetScroll: false to navigate calls in brand.tsx - Fix potential issue where invalid deep links now properly clear selection Co-Authored-By: john@hyprnote.com <john@hyprnote.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughTwo view routes were updated to improve navigation state management. The about route adds null fallback handling when searches fail and adds resetScroll flags to navigation calls. The brand route introduces a public BrandSearch type, adds a validateSearch hook to Route, and implements URL-based state synchronization with navigation integration on item selection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/src/routes/_view/about.tsx (1)
18-21: Consider inlining the type per coding guidelines.The
AboutSearchtype is only used as the return type forvalidateSearchand isn't shared across files. Per coding guidelines, types should generally be inlined unless shared. However, having the explicit type does improve readability, so this is a minor point.As per coding guidelines, consider inlining:
-type AboutSearch = { - type?: "story" | "founder" | "photo"; - id?: string; -}; - export const Route = createFileRoute("/_view/about")({ component: Component, - validateSearch: (search: Record<string, unknown>): AboutSearch => { + validateSearch: (search: Record<string, unknown>): { type?: "story" | "founder" | "photo"; id?: string } => {apps/web/src/routes/_view/brand.tsx (1)
16-19: Consider inlining the type per coding guidelines.Similar to the
about.tsxfile,BrandSearchis only used locally for thevalidateSearchreturn type. Per coding guidelines, consider inlining the type, though the explicit type does improve readability.As per coding guidelines, consider inlining:
-type BrandSearch = { - type?: "visual" | "typography" | "color"; - id?: string; -}; - export const Route = createFileRoute("/_view/brand")({ component: Component, - validateSearch: (search: Record<string, unknown>): BrandSearch => { + validateSearch: (search: Record<string, unknown>): { type?: "visual" | "typography" | "color"; id?: string } => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/routes/_view/about.tsx(2 hunks)apps/web/src/routes/_view/brand.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/brand.tsxapps/web/src/routes/_view/about.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : Use `motion/react` instead of `framer-motion`.
Applied to files:
apps/web/src/routes/_view/brand.tsx
🧬 Code graph analysis (2)
apps/web/src/routes/_view/brand.tsx (2)
apps/web/src/routes/_view/about.tsx (1)
Route(23-45)apps/web/src/routes/_view/index.tsx (1)
Route(115-117)
apps/web/src/routes/_view/about.tsx (1)
plugins/windows/src/ext.rs (1)
navigate(19-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: ci
- GitHub Check: fmt
- GitHub Check: Devin
🔇 Additional comments (8)
apps/web/src/routes/_view/about.tsx (3)
25-35: LGTM! Proper input validation.The
validateSearchimplementation correctly validates and sanitizes search parameters, ensuring only valid type values and string IDs are accepted.
159-179: LGTM! Robust URL-to-state synchronization.The useEffect properly syncs URL search params to local state with defensive null checks when items aren't found. Lines 166-167 and 173-174 correctly handle missing founders/photos by setting state to null rather than leaving stale data.
181-198: LGTM! Consistent scroll preservation.The
handleSetSelectedItemfunction correctly:
- Updates local state immediately for responsive UI
- Syncs state to URL with
resetScroll: falseto preserve scroll position (per PR objectives)- Handles all item types consistently
apps/web/src/routes/_view/brand.tsx (5)
1-1: LGTM! Correct imports for URL state management.The additions of
useNavigateanduseEffectimports are necessary for the URL-driven state synchronization implementation.Also applies to: 4-4
23-33: LGTM! Proper search parameter validation.The
validateSearchimplementation correctly validates search parameters for the brand route, ensuring type safety.
143-168: LGTM! Comprehensive URL synchronization.The useEffect correctly handles all three item types (visual, typography, color) with proper null fallback when items aren't found. The defensive programming ensures no stale state remains when an invalid ID is provided.
170-190: LGTM! Consistent state-to-URL navigation.The
handleSetSelectedItemfunction mirrors the pattern inabout.tsx, providing:
- Immediate local state updates for responsive UI
- URL synchronization with
resetScroll: falseto preserve scroll position- Consistent handling across all item types
139-140: LGTM! Clean integration of URL state management.The integration of
useNavigate,Route.useSearch(), andhandleSetSelectedItemseamlessly adds URL-driven state management without disrupting the existing component structure.Also applies to: 201-201
feat(web): preserve scroll position and add query params to about/brand pages
Summary
This PR makes two related improvements to the about and brand pages:
Scroll Position Preservation: Added
resetScroll: falseto allnavigate()calls in both pages so the global scroll position is maintained when navigating between items inside the MockWindow component.Query Params for Brand Page: Added URL query parameter support to brand.tsx (matching the existing pattern in about.tsx) so users can deep link to specific items:
/brand?type=visual&id=icon/brand?type=typography&id=primary-font/brand?type=color&id=stone-600Invalid Deep Link Handling: Added else branches to clear selection when a valid type is provided but the ID doesn't match any item (fixes a potential stale state issue).
Review & Testing Checklist for Human
/aboutor/brand, scroll down the page, then click on items in the MockWindow. The page scroll position should NOT reset to top./brand?type=visual&id=logoand verify the correct item is selected on page load./brand?type=visual&id=nonexistentand verify the selection is cleared (not stuck on stale state).Notes
resetScroll: falsepattern is already used in gallery, templates, blog, and shortcuts pages in this codebase.