Skip to content

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented Aug 27, 2025

Summary by CodeRabbit

  • New Features

    • Added a responsive Navigation component with open/closed states and accessible toggle; exported for use across the library.
    • Navigation integrated into Header and PageLayout examples.
  • Style

    • Implemented mobile/desktop Navigation styling with themed dropdown behavior.
    • Added a reusable border styling mixin.
  • Documentation

    • Added Storybook stories demonstrating Navigation standalone and within Header, plus an expanded PageLayout example.
  • Refactor

    • Header and PageLayout now wrap individual items in element containers, which may affect custom CSS/layout.

@grahamc grahamc force-pushed the grahamc/navigation branch 2 times, most recently from 2f609aa to 8e67b85 Compare August 27, 2025 14:16
@grahamc grahamc force-pushed the grahamc/header branch 12 times, most recently from a70bf05 to 2b455dc Compare September 11, 2025 22:49
Copy link

coderabbitai bot commented Sep 11, 2025

Walkthrough

Adds a new stateful Navigation component with responsive styles and Storybook stories; re-exports Navigation from the root index; updates Header and PageLayout to wrap child elements individually; adds a Sass border mixin; and adds a FleshedOut PageLayout story demonstrating the new Navigation.

Changes

Cohort / File(s) Summary
Navigation component & stories
src/Navigation/index.tsx, src/Navigation/index.scss, src/Navigation/Navigation.stories.tsx, src/index.ts
Adds a stateful Navigation component (props: `initialState?: "open"
Header adjustments
src/Header/index.tsx
Changes HeaderProps.elements type to React.ReactElement[]; replaces <header role="banner"> with <div className="header">; maps elements to per-element wrapper <div>s with keys.
PageLayout updates & story
src/PageLayout/index.tsx, src/PageLayout/PageLayout.stories.tsx
PageLayout now maps panes to per-pane wrapper <div key={...}>; adds FleshedOut story which composes Header, DetSysIcon, ColorSchemeToggle, and Navigation.
Sass mixins
src/sass/_mixins.scss
Adds border($size) mixin that sets border width/style, themed border-color, and radius with a light-mode override.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Header
  participant Navigation
  participant DOM

  rect #e6f2ff
    Header->>Navigation: mount(initialState)
    Navigation->>DOM: render nav.navigation with id, aria-expanded, aria-controls
    Navigation->>DOM: render .navigation__elements (hidden if closed)
  end

  User->>Navigation: click toggle
  Navigation->>Navigation: toggle menuState
  Navigation->>DOM: update class (navigation--open/--closed) and aria-hidden
  DOM-->>User: elements shown/hidden

  opt Header composition
    Header->>DOM: render per-element wrappers for each element
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Create a Header #50 — Edits to src/Header/index.tsx that are directly related (prop type change and element wrapping).
  • PageLayout component #49 — Changes to PageLayout and PageLayout stories that this PR extends with the FleshedOut example.

Suggested reviewers

  • gustavderdrache

Poem

A nibble of nav beneath moonlight,
I hop and toggle, open bright.
Wrappers snug, panes tucked in line,
Sass borders hum — everything's fine.
A happy rabbit hops away, features in mind. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Responsive navigation container" succinctly and accurately summarizes the primary change in this PR — introduction of a responsive Navigation component along with its styles and Storybook examples. It is concise, focused on the main change, and clear enough for a teammate scanning history to understand the PR's purpose.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch grahamc/navigation

Comment @coderabbitai help to get the list of available commands and usage tips.

Base automatically changed from grahamc/header to main September 12, 2025 01:01
Copy link

netlify bot commented Sep 12, 2025

Deploy Preview for determinate-ui ready!

Name Link
🔨 Latest commit 5bbc07d
🔍 Latest deploy log https://app.netlify.com/projects/determinate-ui/deploys/68c61ce89dfdcf000866c5ff
😎 Deploy Preview https://deploy-preview-51--determinate-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

coderabbitai[bot]

This comment was marked as outdated.

@grahamc grahamc force-pushed the grahamc/navigation branch 2 times, most recently from 4ceada5 to fc760de Compare September 12, 2025 02:15
coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
src/PageLayout/PageLayout.stories.tsx (1)

73-73: Add a key to the single pane element (fixes Biome error).

React array child needs a stable key.

-    panes: [<Placeholder height="3em" label="Pane 1" />],
+    panes: [<Placeholder key="pane-1" height="3em" label="Pane 1" />],
🧹 Nitpick comments (1)
src/PageLayout/PageLayout.stories.tsx (1)

56-64: Avoid Storybook hash navigation jumps.

Anchors with href="#" can cause unwanted page jumps in Storybook; prevent default on click.

-              <a key="docs" href="#">
+              <a key="docs" href="#" onClick={(e) => e.preventDefault()}>
                 Docs
               </a>,
-              <a key="flakes" href="#">
+              <a key="flakes" href="#" onClick={(e) => e.preventDefault()}>
                 Flakes
               </a>,
-              <a key="orgs" href="#">
+              <a key="orgs" href="#" onClick={(e) => e.preventDefault()}>
                 Orgs
               </a>,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c234c6c and 5bbc07d.

📒 Files selected for processing (7)
  • src/Header/index.tsx (1 hunks)
  • src/Navigation/Navigation.stories.tsx (1 hunks)
  • src/Navigation/index.scss (1 hunks)
  • src/Navigation/index.tsx (1 hunks)
  • src/PageLayout/PageLayout.stories.tsx (2 hunks)
  • src/PageLayout/index.tsx (1 hunks)
  • src/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/index.ts
  • src/Navigation/index.tsx
  • src/Header/index.tsx
  • src/PageLayout/index.tsx
  • src/Navigation/index.scss
  • src/Navigation/Navigation.stories.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/PageLayout/PageLayout.stories.tsx (1)
src/Placeholder/index.tsx (1)
  • Placeholder (9-29)
🪛 Biome (2.1.2)
src/PageLayout/PageLayout.stories.tsx

[error] 73-73: Missing key property for this element in iterable.

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

(lint/correctness/useJsxKeyInIterable)

🔇 Additional comments (1)
src/PageLayout/PageLayout.stories.tsx (1)

5-8: Imports look correct for the new story usage.

Header, DetSysIcon, ColorSchemeToggle, and Navigation are all used below. No issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant