-
Notifications
You must be signed in to change notification settings - Fork 432
feat(web): add unified /gallery route and /shortcuts SEO route #1982
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
feat(web): add unified /gallery route and /shortcuts SEO route #1982
Conversation
🤖 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:
|
✅ 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. |
📝 WalkthroughWalkthroughAdds a unified Gallery and Shortcuts feature: new listing and detail routes with loaders, dynamic head metadata, MDX rendering, three-column responsive layouts, download/raw-source links, exported Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser)
participant Router as App Router
participant Loader as Route Loader
participant Data as Content Collections (allTemplates/allShortcuts)
participant UI as React Renderer
Browser->>Router: navigate to /_view/gallery/{type}/{slug} or /_view/shortcuts/{slug}
Router->>Loader: invoke loader(params)
Loader->>Data: lookup item by slug (and type if gallery)
Data-->>Loader: return item or null
alt item found
Loader-->>Router: return loaderData (item)
Router-->>Browser: render route with loaderData
Browser->>UI: render LeftSidebar, MainContent (MDX), RightSidebar
UI->>UI: render suggestions, download/raw links, head meta
else not found
Loader-->>Router: throw notFound()
Router-->>Browser: render 404
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-11-24T16:32:23.055ZApplied to files:
📚 Learning: 2025-11-24T16:32:19.706ZApplied to files:
⏰ 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)
🔇 Additional comments (4)
Comment |
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: 6
🧹 Nitpick comments (4)
apps/web/src/routes/_view/gallery/$type.$slug.tsx (2)
68-84: Remove unusedisTemplatevariable.The
isTemplatevariable on line 71 is declared but never used within theComponentfunction. It's correctly used in child components that receivetypeas a prop.function Component() { const data = Route.useLoaderData(); const { type, item } = data; - const isTemplate = type === "template"; return (
154-182: Remove unusedisTemplatevariable in MainContent.Same issue as in Component -
isTemplateis declared on line 161 but not used within MainContent.function MainContent({ type, item, }: { type: GalleryType; item: (typeof allTemplates)[0] | (typeof allShortcuts)[0]; }) { - const isTemplate = type === "template"; - return (apps/web/src/routes/_view/gallery/index.tsx (1)
635-658: Consider adding focus trap for modal accessibility.The modal has good escape key handling and scroll lock, but for full accessibility compliance, consider adding a focus trap to keep keyboard focus within the modal while it's open. Libraries like
@radix-ui/react-dialogorfocus-trap-reactcan help.apps/web/src/routes/_view/shortcuts/index.tsx (1)
309-312: Consider extracting the magic number for sticky positioning.The
top-[85px]value appears to be tied to the header height. If the header changes, this value would need to be updated manually.Consider using a CSS variable or shared constant for the header height to ensure consistency across the codebase.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/src/components/footer.tsx(1 hunks)apps/web/src/components/header.tsx(1 hunks)apps/web/src/routes/_view/gallery/$type.$slug.tsx(1 hunks)apps/web/src/routes/_view/gallery/index.tsx(1 hunks)apps/web/src/routes/_view/shortcuts/$slug.tsx(1 hunks)apps/web/src/routes/_view/shortcuts/index.tsx(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/components/footer.tsxapps/web/src/components/header.tsxapps/web/src/routes/_view/shortcuts/$slug.tsxapps/web/src/routes/_view/gallery/$type.$slug.tsxapps/web/src/routes/_view/gallery/index.tsxapps/web/src/routes/_view/shortcuts/index.tsx
🧬 Code graph analysis (4)
apps/web/src/routes/_view/shortcuts/$slug.tsx (3)
apps/web/src/routes/_view/shortcuts/index.tsx (1)
Route(16-42)apps/web/src/components/download-button.tsx (1)
DownloadButton(5-21)packages/utils/src/cn.ts (1)
cn(20-22)
apps/web/src/routes/_view/gallery/$type.$slug.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/web/src/components/download-button.tsx (1)
DownloadButton(5-21)
apps/web/src/routes/_view/gallery/index.tsx (4)
apps/web/src/routes/_view/gallery/$type.$slug.tsx (1)
Route(12-66)apps/web/src/components/slash-separator.tsx (1)
SlashSeparator(1-8)packages/utils/src/cn.ts (1)
cn(20-22)apps/web/src/components/download-button.tsx (1)
DownloadButton(5-21)
apps/web/src/routes/_view/shortcuts/index.tsx (3)
apps/web/src/components/slash-separator.tsx (1)
SlashSeparator(1-8)packages/utils/src/cn.ts (1)
cn(20-22)apps/web/src/components/download-button.tsx (1)
DownloadButton(5-21)
⏰ 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). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (18)
apps/web/src/components/footer.tsx (1)
158-164: LGTM!Navigation update from "/templates" to "/gallery" is consistent with the header changes and the new gallery route structure.
apps/web/src/components/header.tsx (1)
32-32: LGTM!Navigation item updated to point to the new gallery route, consistent with footer changes.
apps/web/src/routes/_view/shortcuts/$slug.tsx (1)
1-65: Well-structured route with proper SEO metadata and error handling.Good use of
notFound()for missing shortcuts, proper meta tags for social sharing, and clean component composition.apps/web/src/routes/_view/gallery/$type.$slug.tsx (1)
10-66: Good route setup with proper type validation and SEO.Well-implemented loader with type guard for "template" | "shortcut", appropriate 404 handling, and comprehensive meta tags for social sharing.
apps/web/src/routes/_view/gallery/index.tsx (2)
19-57: Well-structured route with comprehensive SEO and search validation.Good implementation of
validateSearchfor type-safe URL parameters and dynamic meta tags based on filter state.
135-206: Good use of memoization for filtering logic.The multiple
useMemohooks appropriately optimize the filtering computations, preventing unnecessary recalculations on re-renders.apps/web/src/routes/_view/shortcuts/index.tsx (12)
1-14: LGTM!Imports are well-organized and the
cnutility is correctly imported from@hypr/utilsas per coding guidelines.
16-42: LGTM!Route configuration with search validation and SEO metadata is well-structured.
151-174: LGTM!Good use of
cn()with array syntax as per coding guidelines, and proper external link attributes.
176-211: LGTM!Props are correctly inlined in the function signature as per coding guidelines.
213-221: LGTM!Simple presentational component with appropriate styling.
223-263: LGTM!Proper use of
cn()with array syntax for conditional styling. Props correctly inlined.
265-296: LGTM!Good component composition with props correctly passed through.
353-390: LGTM!Good empty state handling and clean grid implementation.
392-420: LGTM!Clean card component with proper button semantics for interactivity.
422-447: LGTM!Proper use of
cn()and external link attributes.
449-469: LGTM!Clean CTA section layout.
543-555: LGTM!Clean helper function for grouping shortcuts by category.
| const handleItemClick = (item: GalleryItem) => { | ||
| setSelectedItem(item); | ||
| window.history.pushState( | ||
| {}, | ||
| "", | ||
| `/gallery/${item.type}/${item.type === "template" ? item.item.slug : item.item.slug}`, | ||
| ); | ||
| }; |
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.
Redundant ternary expression and consider using router navigation.
Line 97 has a redundant ternary where both branches are identical. Additionally, using window.history.pushState directly bypasses TanStack Router's state management, which may cause issues with browser navigation or route matching.
const handleItemClick = (item: GalleryItem) => {
setSelectedItem(item);
- window.history.pushState(
- {},
- "",
- `/gallery/${item.type}/${item.type === "template" ? item.item.slug : item.item.slug}`,
- );
+ window.history.pushState({}, "", `/gallery/${item.type}/${item.item.slug}`);
};🤖 Prompt for AI Agents
In apps/web/src/routes/_view/gallery/index.tsx around lines 92 to 99, the
handleItemClick uses a redundant ternary for the slug and calls
window.history.pushState directly; replace the ternary with a single expression
`/gallery/${item.type}/${item.item.slug}` and swap window.history.pushState for
TanStack Router navigation (e.g., use the router's navigate/useNavigate) so
routing state and history are managed correctly; keep the setSelectedItem call
before navigating and, if needed, pass any route params or state through the
router.navigate call.
| head: ({ loaderData }) => { | ||
| const { shortcut } = loaderData!; | ||
| const url = `https://hyprnote.com/shortcuts/${shortcut.slug}`; |
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.
Non-null assertion on loaderData could cause runtime errors.
If the route is accessed directly without going through the loader (e.g., during SSR edge cases or error boundaries), loaderData could be undefined. Consider adding a guard similar to the gallery route.
head: ({ loaderData }) => {
- const { shortcut } = loaderData!;
+ if (!loaderData) return { meta: [] };
+ const { shortcut } = loaderData;
const url = `https://hyprnote.com/shortcuts/${shortcut.slug}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| head: ({ loaderData }) => { | |
| const { shortcut } = loaderData!; | |
| const url = `https://hyprnote.com/shortcuts/${shortcut.slug}`; | |
| head: ({ loaderData }) => { | |
| if (!loaderData) return { meta: [] }; | |
| const { shortcut } = loaderData; | |
| const url = `https://hyprnote.com/shortcuts/${shortcut.slug}`; |
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/shortcuts/$slug.tsx around lines 21 to 23, remove
the non-null assertion on loaderData and add a guard to handle undefined
loaderData (like the gallery route): check if loaderData and loaderData.shortcut
exist and return a safe/default head (or null) when they do not, otherwise build
the url and head using loaderData.shortcut; avoid throwing by using optional
chaining and an early return so the route is safe during SSR/error-boundary
cases.
| function LeftSidebar({ shortcut }: { shortcut: (typeof allShortcuts)[0] }) { | ||
| return ( | ||
| <aside className="hidden lg:block w-64 shrink-0"> | ||
| <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-18 scrollbar-hide"> |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Tailwind config file
fd -n "tailwind.config" -e ts -e js
# Search for pb-18 usage in the codebase
echo "=== Searching for pb-18 usage ==="
rg -n "pb-18" --type=ts --type=tsx
# Check if 18 is defined in Tailwind spacing config
echo "=== Checking Tailwind config for spacing ==="
fd -n "tailwind.config" -e ts -e js -x cat {}Repository: fastrepl/hyprnote
Length of output: 646
🏁 Script executed:
#!/bin/bash
# Find Tailwind config file
echo "=== Finding Tailwind config files ==="
fd "tailwind.config"
# Search for pb-18 usage in TypeScript/TSX files
echo "=== Searching for pb-18 usage ==="
rg "pb-18" -t ts -t tsx
# If config found, read it
echo "=== Reading Tailwind config content ==="
fd "tailwind.config" -x cat {}Repository: fastrepl/hyprnote
Length of output: 4222
🏁 Script executed:
#!/bin/bash
# Search more broadly for pb-18 anywhere in the codebase
echo "=== Searching for pb-18 anywhere (all files) ==="
rg "pb-18"
# Look for tailwind config in apps/web specifically
echo "=== Looking for tailwind config in apps/web ==="
find apps/web -name "tailwind.config*" 2>/dev/null
# Check if the file under review exists and see context around line 70
echo "=== Checking the file under review ==="
if [ -f "apps/web/src/routes/_view/shortcuts/\$slug.tsx" ]; then
sed -n '65,75p' "apps/web/src/routes/_view/shortcuts/\$slug.tsx"
else
find apps/web -name "\$slug.tsx" 2>/dev/null
fiRepository: fastrepl/hyprnote
Length of output: 1340
🏁 Script executed:
#!/bin/bash
# Search for pb-18 in CSS files to see if it's defined somewhere
echo "=== Searching for pb-18 in CSS/SCSS files ==="
rg "pb-18" --type=css --type=scss
# Search for -pb-18 or pb-18 in any global styles
echo "=== Searching for any custom CSS that might define this ==="
rg "pb-18|pb:\s*18|padding-bottom.*18" --type=css --type=scss
# Check for PostCSS or CSS processing config
echo "=== Looking for PostCSS or CSS config ==="
find . -maxdepth 2 -name "postcss.config*" -o -name "globals.css" -o -name "global.css" 2>/dev/null | head -5
# Check if there's a CSS file in apps/web with custom styles
echo "=== Checking for CSS files in apps/web ==="
find apps/web -name "*.css" -o -name "*.scss" 2>/dev/null | head -10Repository: fastrepl/hyprnote
Length of output: 350
🏁 Script executed:
#!/bin/bash
# Read the styles.css file
echo "=== Reading apps/web/src/styles.css ==="
cat apps/web/src/styles.css
# Also check package.json to see Tailwind version
echo -e "\n=== Checking Tailwind version in package.json ==="
rg "tailwindcss" apps/web/package.json || rg "tailwindcss" package.json | head -5Repository: fastrepl/hyprnote
Length of output: 963
Fix invalid Tailwind padding class pb-18 — use pb-16 or pb-20 instead.
pb-18 is not part of Tailwind's spacing scale (which includes 0, 0.5, 1, 1.5, ..., 16, 20, 24, etc., but not 18). This class also appears in three other files (templates/$slug.tsx, gallery/$type.$slug.tsx, blog/$slug.tsx). Replace with a valid value like pb-16 (4rem) or pb-20 (5rem).
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/shortcuts/$slug.tsx around line 70, the div uses an
invalid Tailwind class `pb-18`; replace `pb-18` with a valid spacing class
(recommended `pb-16` or `pb-20`) — e.g., change to `pb-16`. Also update the same
invalid `pb-18` occurrences in templates/$slug.tsx, gallery/$type.$slug.tsx, and
blog/$slug.tsx to use a valid spacing value (preferably `pb-16` for
consistency).
| const handleShortcutClick = (shortcut: (typeof allShortcuts)[0]) => { | ||
| setSelectedShortcut(shortcut); | ||
| window.history.pushState({}, "", `/shortcuts/${shortcut.slug}`); | ||
| }; | ||
|
|
||
| const handleModalClose = useCallback(() => { | ||
| setSelectedShortcut(null); | ||
| const url = selectedCategory | ||
| ? `/shortcuts?category=${encodeURIComponent(selectedCategory)}` | ||
| : "/shortcuts"; | ||
| window.history.pushState({}, "", url); | ||
| }, [selectedCategory]); |
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.
Use TanStack Router's navigate instead of window.history.pushState.
Direct manipulation of browser history via window.history.pushState bypasses TanStack Router's state management, which can lead to routing inconsistencies and break router features like scroll restoration and navigation guards.
Consider using the router's navigation methods consistently:
const handleShortcutClick = (shortcut: (typeof allShortcuts)[0]) => {
setSelectedShortcut(shortcut);
- window.history.pushState({}, "", `/shortcuts/${shortcut.slug}`);
+ navigate({ to: `/shortcuts/${shortcut.slug}`, replace: false });
};
const handleModalClose = useCallback(() => {
setSelectedShortcut(null);
- const url = selectedCategory
- ? `/shortcuts?category=${encodeURIComponent(selectedCategory)}`
- : "/shortcuts";
- window.history.pushState({}, "", url);
+ navigate({ search: selectedCategory ? { category: selectedCategory } : {}, replace: false });
}, [selectedCategory]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/shortcuts/index.tsx around lines 58 to 69, replace
the direct window.history.pushState calls with TanStack Router navigation:
import and use the router's navigate (e.g., useNavigate or useRouter's navigate)
and call navigate({ to: `/shortcuts/${shortcut.slug}` }) in handleShortcutClick
and navigate({ to: url }) in handleModalClose; include the navigate function in
the useCallback dependency array so routing state, scroll restoration and guards
are handled by TanStack Router instead of mutating history directly.
| return ( | ||
| <div className="fixed inset-0 z-50"> | ||
| <div | ||
| className="absolute inset-0 bg-black/50 backdrop-blur-sm" | ||
| onClick={onClose} | ||
| /> | ||
| <div className="absolute inset-4 sm:inset-8 lg:inset-16 flex items-start justify-center overflow-y-auto"> | ||
| <div | ||
| className={cn([ | ||
| "relative w-full max-w-2xl my-8", | ||
| "bg-[url('/api/images/texture/white-leather.png')]", | ||
| "rounded-sm shadow-2xl", | ||
| ])} | ||
| onClick={(e) => e.stopPropagation()} |
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.
Add accessibility attributes to the modal.
The modal is missing ARIA attributes required for screen reader accessibility: role="dialog", aria-modal="true", and aria-labelledby to reference the title.
- <div className="fixed inset-0 z-50">
+ <div className="fixed inset-0 z-50" role="dialog" aria-modal="true" aria-labelledby="shortcut-modal-title">
<div
className="absolute inset-0 bg-black/50 backdrop-blur-sm"
onClick={onClose}
+ aria-hidden="true"
/>Also add the id to the title element:
- <h2 className="font-serif text-2xl sm:text-3xl text-stone-700 mb-3">
+ <h2 id="shortcut-modal-title" className="font-serif text-2xl sm:text-3xl text-stone-700 mb-3">Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/shortcuts/index.tsx around lines 478 to 491, the
modal markup lacks ARIA attributes: add role="dialog", aria-modal="true", and
aria-labelledby="<unique-id>" to the modal container element, and add a matching
id="<unique-id>" attribute to the title/header element so screen readers can
locate the dialog label; ensure the chosen id is unique in the DOM and update
any references accordingly.
| <button | ||
| onClick={onClose} | ||
| className="absolute top-4 right-4 w-8 h-8 flex items-center justify-center rounded-full bg-white/80 hover:bg-white text-stone-600 hover:text-stone-800 transition-colors cursor-pointer z-10" | ||
| > | ||
| <Icon icon="mdi:close" className="text-lg" /> | ||
| </button> |
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.
Add aria-label to the close button.
The close button lacks an accessible label for screen readers.
<button
onClick={onClose}
+ aria-label="Close modal"
className="absolute top-4 right-4 w-8 h-8 flex items-center justify-center rounded-full bg-white/80 hover:bg-white text-stone-600 hover:text-stone-800 transition-colors cursor-pointer z-10"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={onClose} | |
| className="absolute top-4 right-4 w-8 h-8 flex items-center justify-center rounded-full bg-white/80 hover:bg-white text-stone-600 hover:text-stone-800 transition-colors cursor-pointer z-10" | |
| > | |
| <Icon icon="mdi:close" className="text-lg" /> | |
| </button> | |
| <button | |
| onClick={onClose} | |
| aria-label="Close modal" | |
| className="absolute top-4 right-4 w-8 h-8 flex items-center justify-center rounded-full bg-white/80 hover:bg-white text-stone-600 hover:text-stone-800 transition-colors cursor-pointer z-10" | |
| > | |
| <Icon icon="mdi:close" className="text-lg" /> | |
| </button> |
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/shortcuts/index.tsx around lines 500 to 505, the
close button markup lacks an accessible label for screen readers; add an
aria-label attribute (e.g., aria-label="Close") to the <button> element so
assistive technologies can announce its purpose (you can also include
type="button" if not present to prevent default form submission behavior).
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
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
🧹 Nitpick comments (3)
apps/web/src/routes/_view/gallery/$type.$slug.tsx (3)
44-64: Consider handling undefineddescriptionin meta tags.If
item.descriptionis undefined or empty, the description meta tags will have empty/undefined content, which may not be ideal for SEO. Additionally, consider adding a canonical link tag since the URL is already constructed.return { meta: [ { title: `${item.title} - ${typeLabel} - Hyprnote` }, - { name: "description", content: item.description }, + ...(item.description ? [{ name: "description", content: item.description }] : []), { property: "og:title", content: `${item.title} - ${typeLabel}`, }, - { property: "og:description", content: item.description }, + ...(item.description ? [{ property: "og:description", content: item.description }] : []), { property: "og:type", content: "article" }, { property: "og:url", content: url }, { property: "og:image", content: ogImageUrl }, { name: "twitter:card", content: "summary_large_image" }, { name: "twitter:title", content: `${item.title} - ${typeLabel}`, }, - { name: "twitter:description", content: item.description }, + ...(item.description ? [{ name: "twitter:description", content: item.description }] : []), { name: "twitter:image", content: ogImageUrl }, + { rel: "canonical", href: url }, ], };
85-91: Extract the shared item prop type to reduce repetition.The same prop type
{ type: GalleryType; item: (typeof allTemplates)[0] | (typeof allShortcuts)[0] }is repeated across 6 components (LeftSidebar, MainContent, ItemHeader, ItemContent, SuggestedItems, RightSidebar). Since this type is genuinely shared, extracting it would improve maintainability.type GalleryType = "template" | "shortcut"; +type GalleryItemProps = { + type: GalleryType; + item: (typeof allTemplates)[0] | (typeof allShortcuts)[0]; +};Then use
GalleryItemPropsin each component's props.
249-266: Consider using a more robust key for section items.Using
section.titleas the key assumes titles are unique within a template. If two sections have the same title, React will produce warnings and potential rendering issues. Consider using the index or a combination.- {item.sections.map((section, index) => ( - <div - key={section.title} + {item.sections.map((section, index) => ( + <div + key={`${index}-${section.title}`}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/src/components/header.tsx(1 hunks)apps/web/src/routes/_view/gallery/$type.$slug.tsx(1 hunks)apps/web/src/routes/_view/gallery/index.tsx(1 hunks)apps/web/src/routes/_view/shortcuts/$slug.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/routes/_view/gallery/index.tsx
- apps/web/src/routes/_view/shortcuts/$slug.tsx
- apps/web/src/components/header.tsx
🧰 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/gallery/$type.$slug.tsx
🧬 Code graph analysis (1)
apps/web/src/routes/_view/gallery/$type.$slug.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/web/src/components/download-button.tsx (1)
DownloadButton(5-21)
⏰ 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). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (7)
apps/web/src/routes/_view/gallery/$type.$slug.tsx (7)
68-83: LGTM!Clean component structure with proper data flow from the loader to child components. The three-column layout is well organized.
153-179: LGTM!Clean component composition with proper mobile-responsive back link and logical ordering of content sections.
181-226: LGTM!Good use of type narrowing with
"targets" in itemfor conditional template-specific rendering. Responsive typography and proper anchor ID setup for navigation.
279-321: LGTM!Clean implementation of related items filtering and rendering. Proper use of unique slugs as keys and correct TanStack Router Link params.
323-336: LGTM!Simple and consistent footer navigation matching the pattern used elsewhere in the component.
10-10: The suggestion to importGalleryTypefromgallery/index.tsxis invalid—the type is not exported from that file.The review comment's premise is incorrect. While
GalleryTypeis defined inapps/web/src/routes/_view/gallery/index.tsx(line 12), it is never exported and remains locally scoped. Since it cannot be imported from there, the suggestion to refactor is not actionable. Both files independently defining the same type is an acceptable pattern for locally-scoped, non-shared types.
385-390:bg-linear-to-tis a valid Tailwind CSS class.Both
bg-linear-to-t(newer Tailwind v4 syntax) andbg-gradient-to-t(Tailwind v3 syntax) are valid and produce identical CSS (linear-gradient). Your codebase consistently usesbg-linear-to-tthroughout, so no change is required. The button styling at lines 385-390 is correct.
cd99174 to
cb527d5
Compare
- Add /gallery route with type filter (template/shortcut) and category filter - Add /gallery/$type/$slug detail route for both templates and shortcuts - Add /shortcuts route for SEO with dedicated index and detail pages - Update header and footer navigation to use /gallery instead of /templates - Keep existing /templates route for SEO backward compatibility Co-Authored-By: john@hyprnote.com <john@hyprnote.com>
- Fix head function in gallery/index.tsx (use static title instead of search params) - Remove unused isTemplate variables in gallery/$type.$slug.tsx - Rename 'Gallery' to 'Prompt Gallery' in header navigation - Add 'View raw MDX source' button to gallery and shortcuts detail pages Co-Authored-By: john@hyprnote.com <john@hyprnote.com>
Co-Authored-By: john@hyprnote.com <john@hyprnote.com>
Co-Authored-By: john@hyprnote.com <john@hyprnote.com>
d9f666d to
ad5eb76
Compare
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: 1
♻️ Duplicate comments (5)
apps/web/src/routes/_view/shortcuts/$slug.tsx (2)
21-23: Add a guard forloaderDatato prevent potential runtime errors.The non-null assertion on
loaderDatacould cause issues during SSR edge cases or error boundaries. Consider adding a guard similar to the gallery route'sheadfunction.head: ({ loaderData }) => { - const { shortcut } = loaderData!; + if (!loaderData) return { meta: [] }; + const { shortcut } = loaderData; const url = `https://hyprnote.com/shortcuts/${shortcut.slug}`;
70-70: Replace invalid Tailwind classpb-18withpb-16orpb-20.
pb-18is not part of Tailwind's default spacing scale. Usepb-16(4rem) orpb-20(5rem) instead.- <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-18 scrollbar-hide"> + <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-16 scrollbar-hide">apps/web/src/routes/_view/shortcuts/index.tsx (3)
58-69: Use TanStack Router'snavigateinstead ofwindow.history.pushState.Direct manipulation of browser history bypasses TanStack Router's state management, which can break features like scroll restoration and navigation guards. The
navigatefunction is already imported and used for category changes (line 55).const handleShortcutClick = (shortcut: (typeof allShortcuts)[0]) => { setSelectedShortcut(shortcut); - window.history.pushState({}, "", `/shortcuts/${shortcut.slug}`); + navigate({ to: `/shortcuts/${shortcut.slug}`, replace: false }); }; const handleModalClose = useCallback(() => { setSelectedShortcut(null); - const url = selectedCategory - ? `/shortcuts?category=${encodeURIComponent(selectedCategory)}` - : "/shortcuts"; - window.history.pushState({}, "", url); + navigate({ search: selectedCategory ? { category: selectedCategory } : {}, replace: false }); }, [selectedCategory]);
478-491: Add accessibility attributes to the modal.The modal lacks ARIA attributes required for screen reader accessibility.
- <div className="fixed inset-0 z-50"> + <div className="fixed inset-0 z-50" role="dialog" aria-modal="true" aria-labelledby="shortcut-modal-title"> <div className="absolute inset-0 bg-black/50 backdrop-blur-sm" onClick={onClose} + aria-hidden="true" />Also add the
idto the title element at line 516:- <h2 className="font-serif text-2xl sm:text-3xl text-stone-700 mb-3"> + <h2 id="shortcut-modal-title" className="font-serif text-2xl sm:text-3xl text-stone-700 mb-3">
500-505: Addaria-labelto the close button for screen reader accessibility.<button onClick={onClose} + aria-label="Close modal" className="absolute top-4 right-4 w-8 h-8 flex items-center justify-center rounded-full bg-white/80 hover:bg-white text-stone-600 hover:text-stone-800 transition-colors cursor-pointer z-10" >
🧹 Nitpick comments (5)
apps/web/src/utils/sitemap.ts (1)
149-152: Sitemap entry for/shortcutsis consistent; consider whether to also index gallery/detail routesThe new
/shortcutsroute mirrors/templates(priority 0.7, weekly) and fits well in the existing sitemap structure, so this addition looks good.Given the PR’s SEO goals around shortcuts and the new unified gallery, it may also be worth deciding whether
/galleryand/shortcuts/$slug(detail pages) should be included in the sitemap (potentially via a dynamic loader similar to the blog/changelog/doc entries) so they can be discoverable too.apps/web/src/routes/_view/templates/index.tsx (2)
416-418: Targets rendering simplification looks good; confirm non-emptytargetsor add fallbackJoining
template.targetsinto a single comma-separated line is a nice simplification and easier to scan than multiple badges. This does assumetemplate.targetsis always defined and usually non-empty; if any template can havetargets: []ortargetsomitted, you’ll end up with an empty line under the “For” label. If the content schema doesn’t guarantee at least one target, consider a small fallback (e.g. render nothing or a generic label) or a guard around the join.
520-527: Duplicate targets join in modal; consider shared helper/fallback if format changes laterThe inline “For: {template.targets.join(", ")}” in the modal matches the card and keeps the layout compact. Same note as above: this relies on
targetsalways being present, and the join logic is now duplicated between card and modal, so any future change to formatting or fallback behavior will need to be updated in two places. If you expect this to evolve, a tiny helper (or precomputedtargetsLabel) reused in both places would keep things in sync.apps/web/src/routes/_view/shortcuts/index.tsx (1)
92-93: Consider memoizing or movinggetShortcutsByCategoryoutside the component.
getShortcutsByCategory()is called on every render but operates on static data (allShortcuts). Moving it outside the component or wrapping the result inuseMemowould avoid unnecessary recalculation.+const shortcutsByCategory = getShortcutsByCategory(); +const categories = Object.keys(shortcutsByCategory); + function Component() { const navigate = useNavigate({ from: Route.fullPath }); const search = Route.useSearch(); // ... - const shortcutsByCategory = getShortcutsByCategory(); - const categories = Object.keys(shortcutsByCategory);apps/web/src/routes/_view/gallery/$type.$slug.tsx (1)
338-399: Consider extracting shared sidebar components.The
RightSidebarstructure here closely mirrors the one inshortcuts/$slug.tsx. If more detail routes are added, consider extracting a sharedDetailSidebarcomponent that accepts type-specific props.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/src/routeTree.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (15)
apps/web/content-collections.ts(1 hunks)apps/web/content/shortcuts/action-items.mdx(1 hunks)apps/web/content/shortcuts/customer-concerns.mdx(1 hunks)apps/web/content/shortcuts/decision-summary.mdx(1 hunks)apps/web/content/shortcuts/follow-up-email.mdx(1 hunks)apps/web/content/shortcuts/meeting-insights.mdx(1 hunks)apps/web/content/shortcuts/sales-feedback.mdx(1 hunks)apps/web/src/components/footer.tsx(1 hunks)apps/web/src/components/header.tsx(1 hunks)apps/web/src/routes/_view/gallery/$type.$slug.tsx(1 hunks)apps/web/src/routes/_view/gallery/index.tsx(1 hunks)apps/web/src/routes/_view/shortcuts/$slug.tsx(1 hunks)apps/web/src/routes/_view/shortcuts/index.tsx(1 hunks)apps/web/src/routes/_view/templates/index.tsx(2 hunks)apps/web/src/utils/sitemap.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/web/content/shortcuts/sales-feedback.mdx
- apps/web/content/shortcuts/action-items.mdx
- apps/web/content/shortcuts/follow-up-email.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/header.tsx
- apps/web/src/routes/_view/gallery/index.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/web/content-collections.tsapps/web/src/utils/sitemap.ts
**/*.{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/content-collections.tsapps/web/src/utils/sitemap.tsapps/web/src/routes/_view/shortcuts/$slug.tsxapps/web/src/routes/_view/gallery/$type.$slug.tsxapps/web/src/routes/_view/templates/index.tsxapps/web/src/routes/_view/shortcuts/index.tsxapps/web/src/components/footer.tsx
🧠 Learnings (2)
📚 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} : 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.
Applied to files:
apps/web/src/routes/_view/shortcuts/index.tsx
📚 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} : Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) for 99% of cases instead of setError and similar patterns.
Applied to files:
apps/web/src/routes/_view/shortcuts/index.tsx
🧬 Code graph analysis (2)
apps/web/src/routes/_view/shortcuts/$slug.tsx (3)
apps/web/src/routes/_view/shortcuts/index.tsx (1)
Route(16-42)apps/web/src/components/download-button.tsx (1)
DownloadButton(5-21)packages/utils/src/cn.ts (1)
cn(20-22)
apps/web/src/routes/_view/gallery/$type.$slug.tsx (4)
apps/web/src/routes/_view/gallery/index.tsx (1)
Route(25-58)apps/web/src/routes/_view/shortcuts/$slug.tsx (1)
Route(10-49)packages/utils/src/cn.ts (1)
cn(20-22)apps/web/src/components/download-button.tsx (1)
DownloadButton(5-21)
⏰ 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). (5)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (8)
apps/web/src/components/footer.tsx (1)
159-162: Footer link update to/gallerywith "Prompt Gallery" label looks correctThis matches the PR’s navigation spec and is consistent with other
Linkusages in the file. No issues from a routing or UX standpoint.apps/web/content/shortcuts/customer-concerns.mdx (1)
6-10: LGTM!The
targetsmetadata aligns well with the "When to use" section and follows the expected YAML array format for the new optionaltargetsfield in the shortcuts schema.apps/web/content-collections.ts (1)
422-422: LGTM!The optional
targetsfield for shortcuts is appropriate, allowing gradual adoption across shortcut content files while keeping the existing schema backward-compatible.apps/web/src/routes/_view/shortcuts/$slug.tsx (1)
165-200: LGTM!The
SuggestedShortcutscomponent correctly filters shortcuts by category and excludes the current item, with proper early return for empty results.apps/web/content/shortcuts/decision-summary.mdx (1)
6-10: LGTM!The
targetsmetadata appropriately captures the meeting types where this decision summary shortcut would be most useful.apps/web/content/shortcuts/meeting-insights.mdx (1)
6-10: LGTM!The
targetsvalues appropriately reflect the strategic and analytical nature of this meeting insights shortcut.apps/web/src/routes/_view/gallery/$type.$slug.tsx (2)
35-36: Good defensive handling ofloaderData.The guard
if (!loaderData) return { meta: [] };prevents runtime errors during SSR edge cases, unlike the similar route inshortcuts/$slug.tsx.
212-223: LGTM!The conditional rendering of targets with proper type narrowing (
isTemplate && "targets" in item) correctly handles the different data shapes between templates and shortcuts.
| }) { | ||
| return ( | ||
| <aside className="hidden lg:block w-64 shrink-0"> | ||
| <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-18 scrollbar-hide"> |
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.
Replace invalid Tailwind class pb-18 with pb-16 or pb-20.
pb-18 is not part of Tailwind's default spacing scale. This same issue exists in shortcuts/$slug.tsx.
- <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-18 scrollbar-hide">
+ <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-16 scrollbar-hide">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-18 scrollbar-hide"> | |
| <div className="sticky top-[69px] max-h-[calc(100vh-69px)] overflow-y-auto px-4 pt-6 pb-16 scrollbar-hide"> |
🤖 Prompt for AI Agents
In apps/web/src/routes/_view/gallery/$type.$slug.tsx around line 94 the div uses
an invalid Tailwind class `pb-18`; replace `pb-18` with a valid spacing class
such as `pb-16` or `pb-20` to match the design; also apply the same replacement
in shortcuts/$slug.tsx where the same invalid class is used.
feat(web): add unified /gallery route and /shortcuts SEO route
Summary
This PR introduces a unified
/galleryroute that combines templates and shortcuts browsing, along with a dedicated/shortcutsroute for SEO purposes.New routes added:
/gallery- Unified browsing with?type=templateor?type=shortcutfilters and category filtering/gallery/$type/$slug- Detail pages for both templates and shortcuts/shortcuts- Dedicated shortcuts listing for SEO/shortcuts/$slug- Dedicated shortcut detail pages for SEONavigation updates:
/gallery(positioned above Workflows in the Features menu)/gallerywith label "Prompt Gallery"/templatesroute is preserved for backward compatibility and SEOThe gallery displays both templates (blue badges) and shortcuts (purple badges) with type toggle buttons, search, and category filtering.
Updates since last revision
headfunction in gallery/index.tsx to use static title (TanStack Router'sheaddoesn't have access to search params)isTemplatevariable declarationsReview & Testing Checklist for Human
pnpm -F web devand visit/gallery,/gallery?type=template,/gallery?type=shortcut,/shortcuts, and individual detail pages?plain=1Recommended test plan:
/gallery/gallery/template/standup) and verify the "View raw MDX source" button works/shortcutsdirectly and verify it shows only shortcuts/templatesstill works as beforeNotes
/templatesroute is intentionally preserved for SEO backward compatibilitytargets/sections, shortcuts haveprompt), so detail views render differently based on typeheadfunction doesn't support search params directlyRequested by: john@hyprnote.com (@ComputelessComputer)
Devin session: https://app.devin.ai/sessions/401e07be1d134db1a47bf1f10b446157