fix: Hide SideNav avatar and socials on about page #122
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSideNavPartial.vue adds route-aware logic: introduces Changes
Sequence Diagram(s)sequenceDiagram
participant Router
participant SideNav as SideNavPartial
participant ViewRender as Renderer
Router->>SideNav: provide current route
Note right of SideNav: compute flags\nisHome, isAbout,\nCONTENT_ALIGNED_ROUTES,\nPOST_ROUTE_PREFIX
SideNav->>SideNav: compute navPaddingTopClass
SideNav->>SideNav: evaluate avatarVisible = !isHome && !isAbout
SideNav->>SideNav: evaluate showSocial = !isAbout
SideNav->>ViewRender: render nav with navPaddingTopClass
alt avatarVisible
SideNav->>ViewRender: render Avatar
else avatar hidden
Note right of ViewRender: Avatar omitted
end
alt showSocial
SideNav->>ViewRender: render Social links
else social hidden
Note right of ViewRender: Social omitted (about page)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
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 |
Summary of ChangesHello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the user experience by implementing conditional rendering for elements within the SideNav component. Specifically, it ensures that both the user avatar and social media links are hidden when the user is on the 'About' page, leading to a more focused and contextually appropriate display. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly implements the hiding of the side navigation avatar and social links on the /about page by introducing an isAbout computed property and updating the template conditions. The changes are logical and directly address the requirements.
However, the unit tests have not been updated to reflect these changes. One existing test will now fail, and there is no test coverage for the new logic of hiding social links. I've added comments with details on how to address these critical testing gaps to ensure the codebase remains robust and prevent future regressions.
![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 (1)
src/partials/SideNavPartial.vue (1)
121-121: LGTM! Consider adding a clarifying comment.The
isAboutcomputed property is implemented correctly and consistently with theisHomepattern.For consistency with the
isHomecomputed property (lines 115-119), consider adding a brief comment explaining the use ofpathinstead offullPath:+// `path` excludes query strings, ensuring correct route detection +// even when query parameters are present (e.g. `/about?foo=bar`). const isAbout = computed<boolean>(() => currentRoute.path === '/about');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/partials/SideNavPartial.vue(3 hunks)
🔇 Additional comments (2)
src/partials/SideNavPartial.vue (2)
5-5: LGTM! Avatar correctly hidden on About page.The avatar visibility logic now correctly hides the avatar on both the home page and the About page, as intended.
68-87: LGTM! Social links properly hidden on About page.The social links section is now conditionally rendered and correctly hidden on the About page using the
isAboutflag.
gocanto
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/partials/SideNavPartial.vue (1)
123-138: LGTM! Route-aware padding logic is well-structured.The padding logic correctly handles different route types using a Set for efficient lookup. The default case provides good defensive coding for potential future routes.
One minor observation: the default case (line 137) returns the same value as the home route case (line 130). While this is good defensive coding, you could simplify by removing the home check and relying on the default. However, the explicit home case makes the intent clearer, so the current approach is perfectly fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/partials/SideNavPartial.vue(3 hunks)tests/partials/SideNavPartial.test.ts(2 hunks)
🔇 Additional comments (8)
src/partials/SideNavPartial.vue (4)
5-5: LGTM! Avatar correctly hidden on about page.The avatar visibility logic now properly hides the avatar on both home and about pages as required. The past review comment regarding the broken test has been addressed in the updated test file.
13-13: LGTM! Dynamic padding binding implemented correctly.The nav element now applies route-aware padding through the
navPaddingTopClasscomputed property, providing consistent layout alignment across different page types.
68-87: LGTM! Social links correctly hidden on about page.The conditional rendering with
v-if="!isAbout"properly suppresses social links on the about page as required. The past review comment regarding missing test coverage has been addressed with comprehensive tests in the test file.
121-121: LGTM! Clean implementation of route check.The
isAboutcomputed property follows the same pattern asisHomeand correctly identifies the about route.tests/partials/SideNavPartial.test.ts (4)
39-39: LGTM! Post route added for comprehensive testing.Adding the
/post/:slugroute enables proper testing of thePOST_ROUTE_PREFIXlogic in the component.
96-124: LGTM! Comprehensive avatar visibility tests.The test suite now properly covers:
- Avatar hidden on home route (existing)
- Avatar hidden on about route (new, addresses past review comment)
- Avatar shown on other non-home routes (updated to test multiple paths)
The tests are well-structured and thoroughly validate the avatar visibility logic.
104-136: LGTM! Thorough social links testing.Both tests comprehensively verify that social links are not rendered on the about page:
- Lines 104-112: Direct check that social links are absent
- Lines 126-136: Additional verification that the separator is also absent and that
fetchSocialis still calledThese tests address the past review comment about missing test coverage. While there's some overlap between the two tests, they validate different aspects of the behavior and provide good regression protection.
138-163: LGTM! Excellent padding alignment tests.The tests thoroughly validate the route-aware padding logic:
- Home route uses default padding
pt-16- Content-aligned routes (about, projects, resume, post) use responsive padding
pt-12 md:pt-16The use of loops to test multiple paths ensures consistent behavior across similar route types.
gocanto
deleted the
gus/hide-social-links-and-avatar-on-about-page-2025-10-17
branch
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f1af1c46d48333a3607c19811aa55b
Summary by CodeRabbit