-
Notifications
You must be signed in to change notification settings - Fork 433
Improved anchor navigation with scroll margin and simplified section handling #2061
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
Conversation
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughMultiple route component files are refactored to simplify DOM structure by removing absolute-positioned spacer divs and relative wrapper containers, moving anchor IDs directly onto section elements, and introducing a global scroll-margin-top CSS rule to maintain scroll offset behavior. 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. |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/_view/product/ai-notetaking.tsx (1)
2384-2395: UsefloatingPanelTabs.lengthinstead oftabs.lengthin border conditionIn
FloatingPanelDesktop, the list item border condition usesindex < tabs.length - 1, but the map is overfloatingPanelTabs. This currently works only because both arrays happen to have the same length, but it's misleading and fragile if either changes.- index < tabs.length - 1 && "border-b border-neutral-100", + index < floatingPanelTabs.length - 1 && + "border-b border-neutral-100",
🧹 Nitpick comments (1)
apps/web/src/styles.css (1)
13-17: Global scroll offset for all anchors looks good; consider scope if future ids proliferateApplying
scroll-margin-top: 69pxto[id]in the base layer is a clean way to fix header overlap for in-page anchors and aligns with the section refactors. If you later add many non-anchorids (e.g., intra-component targets), you may want to narrow this selector to something likesection[id]to avoid surprising scroll offsets.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/routes/_view/index.tsx(1 hunks)apps/web/src/routes/_view/product/ai-assistant.tsx(3 hunks)apps/web/src/routes/_view/product/ai-notetaking.tsx(6 hunks)apps/web/src/routes/_view/product/mini-apps.tsx(5 hunks)apps/web/src/styles.css(1 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/index.tsxapps/web/src/routes/_view/product/ai-notetaking.tsxapps/web/src/routes/_view/product/ai-assistant.tsxapps/web/src/routes/_view/product/mini-apps.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
Applied to files:
apps/web/src/routes/_view/product/mini-apps.tsx
⏰ 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). (8)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci
🔇 Additional comments (4)
apps/web/src/routes/_view/index.tsx (1)
292-295: Hero section id refactor aligns with new anchor patternMoving the
heroid onto the<section>itself simplifies the DOM and works nicely with the global scroll margin offset; no issues spotted.apps/web/src/routes/_view/product/ai-assistant.tsx (1)
78-186: Section ids for meeting phases are consistent and ready for anchor navigationAttaching
id="before-meeting",id="during-meeting", andid="after-meeting"directly to the section wrappers is a good simplification and matches the new scroll-margin behavior. Content and borders stay intact, so this should be a pure structural win.Also applies to: 192-292, 298-400
apps/web/src/routes/_view/product/mini-apps.tsx (1)
78-248: Mini-app section ids line up with cross-page linksDefining
id="contacts",id="calendar",id="daily-notes",id="noteshelf", andid="advanced-search"directly on the section elements keeps the layout unchanged while making anchor navigation straightforward. These ids are descriptive and consistent with the existing deep links into the mini-apps page.Also applies to: 254-408, 414-506, 512-600, 630-675
apps/web/src/routes/_view/product/ai-notetaking.tsx (1)
188-287: Ids on AI‑notetaking feature sections are coherent and match incoming linksAdding
id="editor",id="transcription",id="summaries",id="search",id="sharing", andid="floating-panel"directly to the main sections keeps structure simple and makes anchors like/product/ai-notetaking#editorand#searchwork cleanly with the new global scroll offset. No structural regressions spotted.Also applies to: 666-768, 835-1085, 1097-1103, 1940-2125, 2164-2167
Overview
Details