Skip to content

feat: use navigation builder for desktop and mobile links#1235

Merged
danielroe merged 5 commits intonpmx-dev:mainfrom
bdbch:refactor/app-header-navigation-builder
Feb 8, 2026
Merged

feat: use navigation builder for desktop and mobile links#1235
danielroe merged 5 commits intonpmx-dev:mainfrom
bdbch:refactor/app-header-navigation-builder

Conversation

@bdbch
Copy link
Contributor

@bdbch bdbch commented Feb 8, 2026

This PR moves the navigation into it's own configuration object to make the configuration for both - desktop & mobile easier & less prone for errors.

@vercel
Copy link

vercel bot commented Feb 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 8, 2026 10:31pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 8, 2026 10:31pm
npmx-lunaria Ignored Ignored Feb 8, 2026 10:31pm

Request Review

@bdbch
Copy link
Contributor Author

bdbch commented Feb 8, 2026

Looking into the Type errors right now.

@codecov
Copy link

codecov bot commented Feb 8, 2026

Codecov Report

❌ Patch coverage is 19.04762% with 17 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Header/MobileMenu.client.vue 0.00% 10 Missing ⚠️
app/components/AppHeader.vue 36.36% 5 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 8, 2026

📝 Walkthrough

Walkthrough

The PR introduces typed navigation types in app/types/navigation.ts and re-exports them via app/types/index.ts. AppHeader.vue gains computed desktopLinks (NavigationConfig) and mobileLinks (NavigationConfigWithGroups), replaces hard-coded desktop navigation and per-link keyboard handlers with data-driven logic, and passes mobileLinks into the mobile menu. Header/MobileMenu.client.vue now accepts a links prop of type NavigationConfigWithGroups and renders grouped navigation (including separators and optional group labels) dynamically instead of static link blocks.

Possibly related PRs

  • fix: reorder Mobile Menu #1185: Modifies app/components/Header/MobileMenu.client.vue — overlaps with this PR’s changes to make the mobile menu data-driven and reorder its structure.

Suggested labels

front

Suggested reviewers

  • danielroe
  • serhalp
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description accurately describes the changeset, explaining that navigation configuration has been refactored into a dedicated object structure for both desktop and mobile.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉


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

Copy link
Contributor

@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: 5

🧹 Nitpick comments (1)
app/types/navigation.ts (1)

15-17: Consider adding an optional identifier to NavigationSeparator.

Adding a name property to NavigationSeparator would enable consistent keying in Vue templates (as seen with NavigationGroup.name) and avoid relying on array indices.

♻️ Suggested enhancement
 export type NavigationSeparator = {
+  name?: string
   type: 'separator'
 }

Copy link
Contributor

@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: 1

Copy link
Contributor

@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: 1

@danielroe danielroe added this pull request to the merge queue Feb 8, 2026
Merged via the queue into npmx-dev:main with commit 98c68f5 Feb 8, 2026
17 checks passed
@github-actions
Copy link

github-actions bot commented Feb 8, 2026

Thanks for your first contribution, @bdbch! 💫

We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role.

@bdbch bdbch deleted the refactor/app-header-navigation-builder branch February 8, 2026 23:35
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.

2 participants