-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Back up Button tweaks + uses #102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Back up Button tweaks + uses #102
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. WalkthroughAdds a configurable BackToTopLink component and anchors (ProjectsPage and Resume-related partials), wires new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ProjectsPage
participant BackToTopLink
participant Browser
Note over ProjectsPage: Render phase
ProjectsPage->>BackToTopLink: mount with target="#projects-top"
User->>BackToTopLink: click back-to-top
BackToTopLink->>Browser: navigate to anchor (#projects-top)
Browser-->>User: scrolls to anchored heading
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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 focuses on enhancing the user experience by updating the content of the social media widget and improving navigation on the projects page. It modernizes the messaging for social platforms and introduces a convenient 'Back to top' feature, making the site more user-friendly and current. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully updates the copy for the social widget and its tests. It also introduces a new reusable BackToTopLink component and adds it to the ProjectsPage. My review focuses on a small improvement in the implementation of this new component to make it more concise and maintainable by following Vue 3 best practices.
There was a problem hiding this 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/partials/BackToTopLink.vue (1)
11-26: Simplify by removing unnecessarytoRefs.This issue has already been identified in a previous review comment.
🧹 Nitpick comments (2)
src/partials/ExperiencePartial.vue (1)
46-55: Simplify by removing unnecessarytoRefs.In Vue 3's
<script setup>, props are automatically reactive and can be accessed directly in the template. The use oftoRefsadds unnecessary complexity.You can simplify to:
-import { toRefs } from 'vue'; import BackToTopLink from '@partials/BackToTopLink.vue'; import type { ExperienceResponse } from '@api/response/index.ts'; const props = defineProps<{ experience: Array<ExperienceResponse>; backToTopTarget: string; }>(); - -const { backToTopTarget } = toRefs(props);Then update the template to use
props.backToTopTarget:-<BackToTopLink :target="backToTopTarget" /> +<BackToTopLink :target="props.backToTopTarget" />src/partials/RecommendationPartial.vue (1)
33-58: Simplify by removing unnecessarytoRefs.Props are automatically reactive in Vue 3's
<script setup>and can be accessed directly. ThetoRefsusage adds unnecessary complexity.You can simplify to:
-import { computed, toRefs } from 'vue'; +import { computed } from 'vue'; import DOMPurify from 'dompurify'; import BackToTopLink from '@partials/BackToTopLink.vue'; import { image, date } from '@/public.ts'; import type { RecommendationsResponse } from '@api/response/recommendations-response.ts'; import { renderMarkdown } from '@/support/markdown.ts'; const props = defineProps<{ recommendations: Array<RecommendationsResponse>; backToTopTarget: string; }>(); -const { recommendations, backToTopTarget } = toRefs(props); - const processedRecommendations = computed(() => { - return recommendations.value.map((item) => { + return props.recommendations.map((item) => { const sanitisedHtml = DOMPurify.sanitize(renderMarkdown(item.text)); return { ...item, html: sanitisedHtml, formattedDate: date().format(new Date(item.created_at)), }; }); });Then update the template to use
props.backToTopTarget:-<BackToTopLink :target="backToTopTarget" /> +<BackToTopLink :target="props.backToTopTarget" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/pages/ResumePage.vue(1 hunks)src/partials/BackToTopLink.vue(1 hunks)src/partials/ExperiencePartial.vue(2 hunks)src/partials/RecommendationPartial.vue(2 hunks)tests/partials/ExperiencePartial.test.ts(1 hunks)tests/partials/RecommendationPartial.test.ts(1 hunks)
🔇 Additional comments (7)
tests/partials/RecommendationPartial.test.ts (1)
32-34: LGTM!The test correctly provides the new
backToTopTargetprop that the component now requires.src/pages/ResumePage.vue (3)
18-18: LGTM!The anchor target is correctly added to the page title, enabling the back-to-top functionality.
38-40: LGTM!The
back-to-top-targetprop is correctly passed to both partials with a consistent anchor reference.
44-44: LGTM!The BackToTopLink is correctly configured with the matching anchor target.
tests/partials/ExperiencePartial.test.ts (1)
25-25: LGTM!The test correctly provides the new
backToTopTargetprop that the component now requires.src/partials/ExperiencePartial.vue (1)
5-5: LGTM!The
backToTopTargetprop is correctly bound to the BackToTopLink component.src/partials/RecommendationPartial.vue (1)
5-5: LGTM!The
backToTopTargetprop is correctly bound to the BackToTopLink component.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68e356ca912883338da98b17349e0041
Summary by CodeRabbit
New Features
Style
Tests