chore: Enhance UX - II #163
Conversation
|
Caution Review failedThe pull request is closed. 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. WalkthroughThis PR centralizes fade transition CSS into a global stylesheet, removes scoped fade rules from a component, applies fade transitions across many pages/partials to animate skeleton↔content swaps, enhances RecommendationPartial with highlight.js-based syntax highlighting and dark-mode CSS switching, and adds social-links loading state and icon styling in SideNavPartial. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Page
participant Fetch
participant DOM
participant CSS
participant Highlighter
participant SideFetch
User->>Page: navigate / load
Page->>DOM: render skeleton (key="skeleton")
DOM->>CSS: apply fade-enter (fade-enter-from -> fade-enter-active)
Page->>Fetch: request page data
Note right of SideFetch: social links lifecycle
Page->>SideFetch: fetch social links (onMounted)
SideFetch-->>Page: social links or empty
Page->>DOM: update social section (skeleton -> links)
Fetch-->>Page: data arrives
Page->>DOM: swap key -> "content"
DOM->>CSS: fade skeleton out, fade content in
Page->>Highlighter: initializeHighlighter (if content contains code)
Highlighter-->>DOM: highlight code blocks
DOM->>User: content with transitions and highlighted code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (9)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/partials/RecommendationPartial.vue (2)
75-100: Remove redundant initialization call.The
initializeHighlighteris called both in the watcher withimmediate: true(line 83) and inonMounted(line 99). Since the watcher runs immediately on component mount, theonMountedcall is redundant.Apply this diff to remove the redundant initialization:
-onMounted(async () => { - await initializeHighlighter(highlight); -});
67-73: Consider preloading highlight.js styles to prevent FOUC.Dynamic imports of CSS stylesheets can cause a flash of unstyled content (FOUC) when switching between light and dark modes. Consider preloading both stylesheets or using a different approach to theme switching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/images/recommendation/grant-riggle.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (14)
src/components/AccessibleDialog.vue(0 hunks)src/css/support/blog.css(2 hunks)src/pages/AboutPage.vue(2 hunks)src/pages/HomePage.vue(1 hunks)src/pages/PostPage.vue(1 hunks)src/pages/ProjectsPage.vue(2 hunks)src/pages/ResumePage.vue(2 hunks)src/pages/TagPostsPage.vue(1 hunks)src/partials/ArticlesListPartial.vue(1 hunks)src/partials/FeaturedProjectsPartial.vue(1 hunks)src/partials/RecommendationPartial.vue(2 hunks)src/partials/SideNavPartial.vue(7 hunks)src/partials/TalksPartial.vue(1 hunks)src/support/tags.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/AccessibleDialog.vue
🧰 Additional context used
🪛 GitHub Actions: Unit Tests
src/partials/RecommendationPartial.vue
[error] 99-99: Vitest: No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
🪛 GitHub Check: vitest
src/partials/RecommendationPartial.vue
[failure] 83-83: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ immediate src/partials/RecommendationPartial.vue:83:9
❯ processTicksAndRejections node:internal/process/task_queues:105:5
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
[failure] 99-99: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ src/partials/RecommendationPartial.vue:99:8
❯ node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2861:40
❯ callWithErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:200:19
❯ callWithAsyncErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:207:17
❯ hook.__weh.hook.__weh node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2841:19
❯ flushPostFlushCbs node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:382:28
❯ render node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:6018:7
❯ mount node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:3984:13
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
[failure] 83-83: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ immediate src/partials/RecommendationPartial.vue:83:9
❯ processTicksAndRejections node:internal/process/task_queues:105:5
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
[failure] 99-99: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ src/partials/RecommendationPartial.vue:99:8
❯ node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2861:40
❯ callWithErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:200:19
❯ callWithAsyncErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:207:17
❯ hook.__weh.hook.__weh node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2841:19
❯ flushPostFlushCbs node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:382:28
❯ render node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:6018:7
❯ mount node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:3984:13
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
🔇 Additional comments (9)
src/partials/RecommendationPartial.vue (1)
7-30: Layout refactoring looks good.The new two-column layout structure with avatar and content areas improves maintainability. The use of
post-markdownclass for consistent styling is a good pattern.src/pages/AboutPage.vue (2)
55-62: LGTM!The transition wrapper for the "Let's Connect" section is properly configured with stable keys and appropriate mode settings. This provides a smooth user experience when loading profile data.
74-77: LGTM!The skills widget transition follows the same pattern as the rest of the application and correctly handles the loading-to-content state change.
src/css/support/blog.css (2)
24-26: LGTM!The new icon utility class is straightforward and follows Tailwind conventions.
213-222: LGTM!The fade transition classes are implemented correctly with standard opacity transitions. This provides a consistent animation pattern across all components using the fade transition.
src/partials/ArticlesListPartial.vue (1)
25-33: LGTM!The transition wrapper correctly uses
<transition>(nottransition-group) for conditional rendering with proper keys. The three-state pattern (loading, list, empty) is well-structured.src/pages/HomePage.vue (1)
29-32: LGTM!The skills widget transition is properly implemented with stable keys and follows the established pattern across the application.
src/pages/ResumePage.vue (2)
33-51: LGTM!The main content transition wrapper is well-structured with appropriate keys for skeleton and content states. The computed
shouldShowSkeletonproperty makes the condition clear and maintainable.
64-67: LGTM!The skills widget transition follows the established pattern and correctly handles the loading state.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/PostPage.vue (1)
211-216: Encode share URLs/titles
xURLForand the LinkedIn share link build query strings with rawpost.titleandfullURLFor(post). Titles or URLs containing spaces or&will break the share links. Please URL-encode both parameters before concatenation.Apply this diff:
-const xURLFor = (post: PostResponse) => { - return `https://x.com/intent/tweet?url=${fullURLFor(post)}&text=${post.title}`; -}; +const xURLFor = (post: PostResponse) => { + const shareUrl = encodeURIComponent(fullURLFor(post)); + const shareText = encodeURIComponent(post.title); + return `https://x.com/intent/tweet?url=${shareUrl}&text=${shareText}`; +}; -const fullURLFor = (post: PostResponse) => siteUrlFor(`/post/${post.slug}`); +const fullURLFor = (post: PostResponse) => siteUrlFor(`/post/${post.slug}`);And in the template:
-:href="`https://www.linkedin.com/sharing/share-offsite/?url=${fullURLFor(post)}`" +:href="`https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent(fullURLFor(post))}`"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/images/recommendation/grant-riggle.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (14)
src/components/AccessibleDialog.vue(0 hunks)src/css/support/blog.css(2 hunks)src/pages/AboutPage.vue(2 hunks)src/pages/HomePage.vue(1 hunks)src/pages/PostPage.vue(1 hunks)src/pages/ProjectsPage.vue(2 hunks)src/pages/ResumePage.vue(2 hunks)src/pages/TagPostsPage.vue(1 hunks)src/partials/ArticlesListPartial.vue(1 hunks)src/partials/FeaturedProjectsPartial.vue(1 hunks)src/partials/RecommendationPartial.vue(2 hunks)src/partials/SideNavPartial.vue(7 hunks)src/partials/TalksPartial.vue(1 hunks)src/support/tags.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/AccessibleDialog.vue
🧰 Additional context used
🪛 GitHub Actions: Unit Tests
src/partials/RecommendationPartial.vue
[error] 99-99: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
[error] 83-83: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
🪛 GitHub Check: vitest
src/partials/RecommendationPartial.vue
[failure] 83-83: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ immediate src/partials/RecommendationPartial.vue:83:9
❯ processTicksAndRejections node:internal/process/task_queues:105:5
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
[failure] 99-99: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ src/partials/RecommendationPartial.vue:99:8
❯ node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2861:40
❯ callWithErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:200:19
❯ callWithAsyncErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:207:17
❯ hook.__weh.hook.__weh node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2841:19
❯ flushPostFlushCbs node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:382:28
❯ render node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:6018:7
❯ mount node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:3984:13
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
[failure] 83-83: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ immediate src/partials/RecommendationPartial.vue:83:9
❯ processTicksAndRejections node:internal/process/task_queues:105:5
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
[failure] 99-99: Unhandled error
Error: [vitest] No "initializeHighlighter" export is defined on the "@/support/markdown.ts" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("@/support/markdown.ts"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VitestMocker.createError node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:284:17
❯ Object.get node_modules/vitest/dist/chunks/execute.B7h3T_Hc.js:330:16
❯ src/partials/RecommendationPartial.vue:99:8
❯ node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2861:40
❯ callWithErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:200:19
❯ callWithAsyncErrorHandling node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:207:17
❯ hook.__weh.hook.__weh node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:2841:19
❯ flushPostFlushCbs node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:382:28
❯ render node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:6018:7
❯ mount node_modules/@vue/runtime-core/dist/runtime-core.cjs.js:3984:13
This error originated in "tests/partials/RecommendationPartial.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
🔇 Additional comments (10)
src/support/tags.ts (1)
38-43: Good normalization approach.Using a dedicated helper to normalize tag parameters before routing ensures consistent URL formatting and prevents case-sensitivity issues in navigation.
src/partials/ArticlesListPartial.vue (1)
25-33: LGTM! Clean transition implementation.The fade transition is properly configured with explicit keys for each state (skeleton, list, empty), mode="out-in" to prevent layout shifts, and appear for initial animation. This provides a smooth UX when toggling between loading and loaded states.
src/css/support/blog.css (2)
24-26: LGTM! Clean utility class.The
blog-side-nav-iconutility provides consistent sizing for side navigation icons.
213-222: LGTM! Standard fade transition implementation.The fade transition classes provide smooth 0.3s opacity animations that are properly utilized across multiple components in this PR. The implementation follows Vue transition naming conventions correctly.
src/pages/HomePage.vue (1)
29-32: LGTM! Proper transition implementation.The fade transition correctly wraps the skills widget with explicit keys and mode="out-in" to ensure smooth animation between skeleton and loaded states.
src/pages/ResumePage.vue (2)
33-51: LGTM! Well-structured content transition.The main resume content is properly wrapped in a fade transition with explicit keys for skeleton and content states, ensuring smooth animation during data loading.
64-67: LGTM! Consistent sidebar transition.The sidebar skills widget follows the same transition pattern used throughout the codebase, providing a consistent UX.
src/pages/ProjectsPage.vue (1)
57-60: LGTM! Consistent sidebar transition.The sidebar skills widget follows the same transition pattern, providing consistent UX across pages.
src/pages/AboutPage.vue (2)
55-62: LGTM! Clean transition for connect section.The "Let's Connect" paragraph is properly wrapped in a fade transition with explicit keys, ensuring smooth animation between loading and loaded states.
74-77: LGTM! Consistent sidebar transition.The sidebar follows the same transition pattern used across other pages, maintaining UX consistency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/partials/RecommendationPartial.test.ts (1)
8-10: Mock should return a resolved Promise to match the real signature.The real
initializeHighlighterfunction is async and returnsPromise<void>(seesrc/support/markdown.ts). The mock currently returnsundefinedby default, which may work but is imprecise. If the component awaits this function, returning a proper Promise prevents potential timing or type issues.Apply this diff to return a resolved Promise:
-const initializeHighlighter = vi.hoisted(() => vi.fn()); +const initializeHighlighter = vi.hoisted(() => vi.fn(() => Promise.resolve()));tests/partials/EducationPartial.test.ts (1)
8-10: Mock should return a resolved Promise to match the real signature.The real
initializeHighlighterfunction is async and returnsPromise<void>(seesrc/support/markdown.ts). The mock currently returnsundefinedby default, which may work but is imprecise. If the component awaits this function, returning a proper Promise prevents potential timing or type issues.Apply this diff to return a resolved Promise:
-const initializeHighlighter = vi.hoisted(() => vi.fn()); +const initializeHighlighter = vi.hoisted(() => vi.fn(() => Promise.resolve()));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/partials/EducationPartial.test.ts(1 hunks)tests/partials/RecommendationPartial.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/partials/EducationPartial.test.ts (1)
src/support/markdown.ts (1)
initializeHighlighter(80-110)
tests/partials/RecommendationPartial.test.ts (1)
src/support/markdown.ts (1)
initializeHighlighter(80-110)
Summary by CodeRabbit
New Features
Style
Bug Fixes
Tests