feat: reorganize footer links into labeled columns#2211
feat: reorganize footer links into labeled columns#2211thealxlabs wants to merge 5 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe desktop footer was changed from a single horizontal flex row to a responsive three-column grid and links were redistributed into three columns labelled Product, Legal (Legal & Info), and Community. The keyboard shortcuts button was restyled for left-aligned text while retaining its modal trigger behaviour. i18n keys for the footer were updated to include Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/components/AppFooter.vue (2)
23-25: Trim structural template comments that restate obvious markup.Line [23], Line [25], Line [122], and Line [134] describe self-evident layout blocks; removing them will keep the template leaner.
As per coding guidelines: "Add comments only to explain complex logic or non-obvious implementations".
Also applies to: 122-122, 134-134
35-35: Prefer global focus-visible styling for this button.Line [35] applies inline
focus-visible:(...)utilities on a<button>. In this codebase, button focus-visible styling should come from the global rule to keep behaviour consistent.Suggested minimal class adjustment
- class="text-start cursor-pointer inline-flex gap-x-1 items-center underline-offset-[0.2rem] underline decoration-1 decoration-fg/30 font-mono text-fg hover:(decoration-accent text-accent) focus-visible:(decoration-accent text-accent) transition-colors duration-200" + class="text-start cursor-pointer inline-flex gap-x-1 items-center underline-offset-[0.2rem] underline decoration-1 decoration-fg/30 font-mono text-fg hover:(decoration-accent text-accent) transition-colors duration-200"Based on learnings: focus-visible styling for button/select elements should be handled globally in
app/assets/main.css, and per-element inline focus-visible utility classes should be avoided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1b18a4a-2880-49a8-a60d-63772fefabfd
📒 Files selected for processing (2)
app/components/AppFooter.vuetest/nuxt/components/AppFooter.spec.ts
| // The Product column should link to /about and /blog | ||
| const aboutLink = component.find('a[href="/about"]') | ||
| expect(aboutLink.exists()).toBe(true) | ||
| }) |
There was a problem hiding this comment.
Test intent and assertions are out of sync.
Line [27] states /about and /blog should be present, but only /about is asserted. Please assert /blog too (or narrow the test description/comment).
Suggested test completion
// The Product column should link to /about and /blog
const aboutLink = component.find('a[href="/about"]')
expect(aboutLink.exists()).toBe(true)
+ const blogLink = component.find('a[href="/blog"]')
+ expect(blogLink.exists()).toBe(true)📝 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.
| // The Product column should link to /about and /blog | |
| const aboutLink = component.find('a[href="/about"]') | |
| expect(aboutLink.exists()).toBe(true) | |
| }) | |
| // The Product column should link to /about and /blog | |
| const aboutLink = component.find('a[href="/about"]') | |
| expect(aboutLink.exists()).toBe(true) | |
| const blogLink = component.find('a[href="/blog"]') | |
| expect(blogLink.exists()).toBe(true) | |
| }) |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
🔗 Linked issue
resolves #1364
🧭 Context
The app footer displayed links in an unorganized flat list. Grouping them into labeled columns (Product, Legal, Community) makes the footer easier to scan and aligns with common web conventions.
📚 Description
Refactored
AppFooter.vueto organize links into three labeled column groups:Column labels use
tracking-wideuppercase text intext-fg-subtlefor a subtle separator. The layout uses a responsive grid (2 columns on sm, 3 on lg).A regression test added to
test/nuxt/components/AppFooter.spec.tsverifies that all three column label spans (Product, Legal, Community) are present in the rendered footer.