feat(brand): Add comprehensive brand assets and details page #1744
Conversation
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughRefactored the Brand page from a static visual assets display into an interactive multi-pane explorer. Introduces new data structures for typography and colors, implements a selection-based detail view workflow with resizable panels, and adds modular UI components for grid and detail views across multiple item categories. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrandPage
participant BrandGridView
participant BrandDetailView
participant DetailPanel
User->>BrandPage: Visit Brand page
BrandPage->>BrandGridView: No item selected
BrandGridView->>User: Display grid of visuals/typography/colors
User->>BrandGridView: Click item
BrandGridView->>BrandPage: setSelectedItem(item)
BrandPage->>BrandDetailView: Render with selected item
BrandDetailView->>DetailPanel: Show resizable detail panel
rect rgba(100, 200, 150, 0.2)
note right of DetailPanel: Detail view active<br/>with sidebar for item type
DetailPanel->>User: Display item details + actions
end
User->>DetailPanel: Click close/deselect
DetailPanel->>BrandPage: setSelectedItem(null)
BrandPage->>BrandGridView: Return to grid view
BrandGridView->>User: Display grid
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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/brand.tsx (2)
115-118: NormalizesetSelectedItemtyping across components to avoid TS frictionRight now
setSelectedItemis:
- A
useState<SelectedItem | null>setter at Line 121,- Typed as
(item: SelectedItem | null) => voidin some props (e.g.,BrandContentSection,BrandDetailView,BrandDetailPanel),- Typed as
(item: SelectedItem) => voidin others (e.g.,BrandGridView,VisualAssetsGrid,TypographyGrid,ColorsGrid, sidebars).Depending on your TS config, passing the React state setter (which is
Dispatch<SetStateAction<SelectedItem | null>>) through these narrower function types can cause assignability issues and is a bit harder to reason about.You can simplify and make everything consistent by introducing a shared alias and using it everywhere:
type SelectedItem = | { type: "visual"; data: (typeof VISUAL_ASSETS)[0] } | { type: "typography"; data: (typeof TYPOGRAPHY)[0] } | { type: "color"; data: (typeof COLORS)[0] }; + +type SetSelectedItem = (item: SelectedItem | null) => void; @@ function Component() { - const [selectedItem, setSelectedItem] = useState<SelectedItem | null>(null); + const [selectedItem, setSelectedItem] = useState<SelectedItem | null>(null); @@ - <BrandContentSection - selectedItem={selectedItem} - setSelectedItem={setSelectedItem} - /> + <BrandContentSection + selectedItem={selectedItem} + setSelectedItem={(item) => setSelectedItem(item)} + /> @@ function BrandContentSection({ selectedItem, setSelectedItem, }: { selectedItem: SelectedItem | null; - setSelectedItem: (item: SelectedItem | null) => void; + setSelectedItem: SetSelectedItem; @@ function BrandGridView({ setSelectedItem, }: { - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function VisualAssetsGrid({ setSelectedItem, }: { - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function TypographyGrid({ setSelectedItem, }: { - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function ColorsGrid({ setSelectedItem, }: { - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function BrandDetailView({ selectedItem, setSelectedItem, }: { selectedItem: SelectedItem; - setSelectedItem: (item: SelectedItem | null) => void; + setSelectedItem: SetSelectedItem; @@ function BrandSidebar({ selectedItem, setSelectedItem, }: { selectedItem: SelectedItem; - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function VisualAssetsSidebar({ selectedItem, setSelectedItem, }: { selectedItem: SelectedItem; - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function TypographySidebar({ selectedItem, setSelectedItem, }: { selectedItem: SelectedItem; - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function ColorsSidebar({ selectedItem, setSelectedItem, }: { selectedItem: SelectedItem; - setSelectedItem: (item: SelectedItem) => void; + setSelectedItem: SetSelectedItem; @@ function BrandDetailPanel({ selectedItem, setSelectedItem, }: { selectedItem: SelectedItem; - setSelectedItem: (item: SelectedItem | null) => void; + setSelectedItem: SetSelectedItem;This keeps the API consistent and sidesteps any weirdness with
Dispatch<SetStateAction<…>>while still allowing null to clear the selection.Also applies to: 120-121, 155-161, 184-188, 198-202, 230-234, 261-265, 292-298, 314-320, 341-347, 388-394, 434-440, 480-486
190-195: Small interaction/accessibility tweaks for buttons and imagesThe grid and sidebar UIs look good and are keyboard-focusable via
<button>, but a couple of small tweaks would tighten things up:
- Add
type="button"to all these non-submit<button>s to avoid accidental form submission if they’re ever rendered inside a<form>.- For the asset images that aren’t critical to initial layout, consider
loading="lazy"to avoid eagerly loading everything on large press-kit pages.For example:
- {VISUAL_ASSETS.map((asset) => ( - <button + {VISUAL_ASSETS.map((asset) => ( + <button + type="button" key={asset.id} @@ - <img - src={asset.url} - alt={asset.name} + <img + src={asset.url} + alt={asset.name} + loading="lazy" className="max-w-full max-h-full object-contain group-hover:scale-110 transition-transform" />You can apply the same
type="button"pattern to the Typography/Colors grids and the three sidebars, andloading="lazy"anywhere the image is not immediately above the fold.Also applies to: 203-227, 235-258, 266-289, 349-384, 395-430, 441-476
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/routes/_view/brand.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/routes/_view/brand.tsx (2)
apps/web/src/components/mock-window.tsx (1)
MockWindow(4-56)packages/utils/src/cn.ts (1)
cn(20-22)
⏰ 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 (3)
apps/web/src/routes/_view/brand.tsx (3)
14-26: Route metadata, hero, and status bar align well with the new Brand pageThe updated
<Route>head, hero copy, andBrandStatusBarmessaging are cohesive and accurately reflect the expanded scope of the Brand page (visuals, typography, colors). No issues from a structure or semantics perspective.Also applies to: 139-153, 688-703
28-118: Data modeling for assets/typography/colors andSelectedItemis cleanUsing
VISUAL_ASSETS,TYPOGRAPHY, andCOLORSarrays plus a discriminatedSelectedItemunion keeps the rest of the UI nicely type-safe and data-driven. This should make it straightforward to extend the catalog later without touching control flow.
120-127: Detail view/resizable layout is well-structured; verify custom gradient classesThe split-pane detail view (
BrandDetailView+BrandSidebar+BrandDetailPanel) and the three detail components (VisualAssetDetail,TypographyDetail,ColorDetail) are well factored and make good use of the resizable and scrollable containers. The close actions correctly callsetSelectedItem(null)so navigation back to the grid is straightforward.One small thing to double-check: the gradient-style classes such as
bg-linear-to-bon the outer wrapper andbg-linear-to-ton the Download button rely on your Tailwind/theme configuration. If your setup only defines the standard Tailwindbg-gradient-to-*utilities, these might be no-ops and you’d want to rename them.- <div - className="bg-linear-to-b from-white via-stone-50/20 to-white min-h-screen" + <div + className="bg-gradient-to-b from-white via-stone-50/20 to-white min-h-screen" @@ - className="px-4 h-8 flex items-center text-sm bg-linear-to-t from-neutral-200 to-neutral-100 text-neutral-900 rounded-full shadow-sm hover:shadow-md hover:scale-[102%] active:scale-[98%] transition-all" + className="px-4 h-8 flex items-center text-sm bg-gradient-to-t from-neutral-200 to-neutral-100 text-neutral-900 rounded-full shadow-sm hover:shadow-md hover:scale-[102%] active:scale-[98%] transition-all"If
bg-linear-to-*is intentional and provided by a custom plugin, then this is already fine as-is.Also applies to: 300-312, 322-338, 480-510, 513-553, 555-639, 641-686
No description provided.