chore: Local development + UX #173
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. WalkthroughUpdate local dev proxy/CORS and docker volumes; remove many Vue wrappers; add client-side text-highlighting and Tag summary object shape; wire tag search to router and back navigation; remove sidebar social links; and adapt many tests to use real router and Pinia. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Header
participant Router
participant TagPage
participant ApiStore
participant API
User->>Header: submit search
Header->>ApiStore: set searchTerm
Header->>Router: push TagPosts route (tag)
Router->>TagPage: route activate
TagPage->>ApiStore: watch(searchTerm, tag)
TagPage->>API: loadPostsForTag(tag, { text: searchTerm })
API-->>TagPage: posts
TagPage->>TagPage: render posts (useTextHighlight for highlights)
sequenceDiagram
autonumber
participant ArticleItem
participant useTextHighlight
participant DOMPurify
ArticleItem->>useTextHighlight: highlight(text)
alt searchTerm present
useTextHighlight->>useTextHighlight: escape regex, wrap matches in <mark>
end
useTextHighlight->>DOMPurify: sanitize HTML
DOMPurify-->>ArticleItem: safe HTML
ArticleItem->>DOM: inject via v-html
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
Comment |
![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".
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/partials/SideNavPartial.test.ts (1)
9-36: Remove unused social-related test fixtures and imports.Since the social links section has been removed from the component, the following are no longer needed:
SocialResponseandApiResponsetype imports (line 9)- Mock social data array (lines 11-33)
getSocialmock function (lines 35-36)Apply this diff to clean up the unused code:
-import type { SocialResponse, ApiResponse } from '@api/response/index.ts'; - -const social: SocialResponse[] = [ - { - uuid: faker.string.uuid(), - name: 'github', - handle: faker.internet.userName(), - url: faker.internet.url(), - description: faker.lorem.words(2), - }, - { - uuid: faker.string.uuid(), - name: 'linkedin', - handle: faker.internet.userName(), - url: faker.internet.url(), - description: faker.lorem.words(2), - }, - { - uuid: faker.string.uuid(), - name: 'x', - handle: faker.internet.userName(), - url: faker.internet.url(), - description: faker.lorem.words(2), - }, -]; - -const getSocial = vi.fn<[], Promise<ApiResponse<SocialResponse[]>>>(() => Promise.resolve({ version: '1.0.0', data: social })); -vi.mock('@api/store.ts', () => ({ useApiStore: () => ({ getSocial }) })); -src/pages/PostPage.vue (1)
197-209: Direct DOM manipulation for search input.The function directly manipulates the search input's value and dispatches synthetic events. While functional, this bypasses Vue's reactivity and could be fragile if the DOM structure changes. Consider using a ref or emitting events to a parent component that owns the search state.
src/partials/HeaderPartial.vue (1)
13-21: Search UX and navigation flow look correct; consider reusing Tags.routeFor
- The conditional SVG for the error state (lines 13–21) cleanly swaps between a clear “X” and the normal search icon, and the
@click="clearSearchAndError"hook is appropriate.- Updated
performSearchcorrectly:
- Resets validation,
- Leaves the store in sync on empty query,
- Enforces the min-length guard, and
- On valid input, sets
apiStore.searchTermand pushes to theTagPostsroute using a lowercased tag.You might consider delegating the route creation to
Tags.routeFor(query)to keep the tag normalization in one place, but that’s optional and not a blocker.Also applies to: 59-60, 66-66, 81-101
tests/partials/HeaderPartial.test.ts (1)
31-38: Strengthen search tests to avoid flakiness and better assert validation
- Line 36:
expect(wrapper.vm.validationError).toBeDefined()doesn't prove the validation error occurred;validationErroris always defined as a ref. Strengthen toexpect(wrapper.vm.validationError).toBeTruthy()or assert the exact message:expect(wrapper.vm.validationError).toBe('Search term must be at least 5 characters.').- Line 42:
faker.lorem.words(2)can produce strings shorter than 5 characters (e.g., "a b" = 3 chars), causing intermittent test failures. Use a fixed string like'valid search'instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (24)
caddy/WebCaddyfile.local(2 hunks)docker-compose.yml(1 hunks)eslint.config.js(1 hunks)src/components/WidgetSkillsTransitionWrapper.vue(1 hunks)src/components/WidgetSocialTransitionWrapper.vue(1 hunks)src/pages/AboutPage.vue(1 hunks)src/pages/PostPage.vue(1 hunks)src/pages/ProjectsPage.vue(1 hunks)src/pages/ResumePage.vue(1 hunks)src/pages/TagPostsPage.vue(7 hunks)src/partials/ArticleItemPartial.vue(2 hunks)src/partials/ArticlesListPartial.vue(2 hunks)src/partials/BackToTopLink.vue(1 hunks)src/partials/FeaturedProjectsPartial.vue(1 hunks)src/partials/HeaderPartial.vue(3 hunks)src/partials/SideNavPartial.vue(0 hunks)src/partials/TalksPartial.vue(1 hunks)src/public.ts(2 hunks)src/support/tags.ts(2 hunks)tests/pages/TagPostsPage.test.ts(5 hunks)tests/partials/ArticleItemPartial.test.ts(2 hunks)tests/partials/HeaderPartial.test.ts(3 hunks)tests/partials/SideNavPartial.test.ts(1 hunks)tests/support/tags.test.ts(5 hunks)
💤 Files with no reviewable changes (1)
- src/partials/SideNavPartial.vue
🧰 Additional context used
🧬 Code graph analysis (3)
tests/pages/TagPostsPage.test.ts (1)
src/stores/api/store.ts (1)
getPosts(114-122)
tests/partials/HeaderPartial.test.ts (1)
src/stores/api/store.ts (1)
setSearchTerm(34-36)
tests/support/tags.test.ts (1)
src/support/tags.ts (1)
Tags(16-77)
🔇 Additional comments (27)
tests/partials/SideNavPartial.test.ts (1)
86-90: LGTM! Test correctly validates the removal of social links.The updated test assertion properly verifies that the social links section no longer exists in the SideNavPartial component.
src/components/WidgetSkillsTransitionWrapper.vue (1)
3-4: LGTM!The transition wrapper has been removed while preserving the conditional rendering logic. This simplifies the component structure and eliminates fade animations as part of the broader UX update.
src/pages/AboutPage.vue (1)
56-61: LGTM!The transition wrapper has been removed from the connect section while maintaining the same conditional rendering logic. This aligns with the PR's objective of simplifying UX by removing transition animations.
src/partials/TalksPartial.vue (1)
7-39: LGTM!The transition wrapper has been removed, adopting direct conditional rendering for the skeleton and talks grid. The rendering logic remains intact, aligning with the PR-wide pattern of simplifying UX transitions.
src/public.ts (2)
1-1: LGTM!The import of VueRouter is appropriate for the new navigation utility function.
56-68: LGTM!The goBack function correctly handles three scenarios:
- SSR environments (no window) fallback to Home
- Client-side with history uses router.back()
- Client-side without history fallback to Home
The logic is sound and defensive.
src/components/WidgetSocialTransitionWrapper.vue (1)
3-4: LGTM!The transition wrapper has been removed while preserving the conditional rendering logic. This aligns with the PR-wide simplification of transition animations.
src/pages/ResumePage.vue (1)
33-49: LGTM!The transition wrapper has been removed and the rendering structure simplified. The skeleton and content branches are now clearly separated, with each section maintaining its own conditional rendering based on data availability. This aligns with the PR's UX simplification goals.
src/support/tags.ts (2)
9-14: LGTM!The new TagSummaryDescription type provides a structured way to represent tag summaries with optional interactive elements. This improves the API surface for handling user interactions with tag labels.
54-76: All callers have been properly updated for the breaking API change.The verification confirms:
- Production usage in
src/pages/TagPostsPage.vue(lines 149-156) correctly receives the structuredTagSummaryDescriptionobject into a computed property- Template code (lines 38-51) properly accesses each property:
text,label,suffix, andonLabelClick- All test cases in
tests/support/tags.test.tsexpect and validate the new structured return type- No code attempts to use the return value as a string
docker-compose.yml (1)
62-62: Port consistency verified across all relay configurations.The change to port 18080 in docker-compose.yml (line 62) is correctly reflected across related configurations:
RELAY_UPSTREAMenvironment variable in docker-compose.yml is set tohttp://host.docker.internal:18080- WebCaddyfile.local uses the
$RELAY_UPSTREAMenvironment variable with the correct fallback- No lingering references to port 8080 in relay contexts
- Configuration is consistent across all files
caddy/WebCaddyfile.local (2)
14-23: LGTM!The CORS header configuration is appropriate for local development, allowing dynamic origins and exposing necessary headers.
27-36: Clear documentation of path transformation logic.The comments effectively explain the multi-step path transformation (/relay/generate-signature → /generate-signature → /api/generate-signature), and the Origin header forwarding ensures downstream CORS middleware functions correctly.
src/pages/PostPage.vue (2)
30-32: LGTM!The loading state handling with skeleton is clean and follows Vue best practices with explicit keys for state transitions.
36-93: Share buttons positioned before the post title.The current structure places share buttons and publication date before the h1 title. While functionally correct, this is unconventional—most blogs lead with the title for better SEO and accessibility. Consider whether this ordering aligns with your UX goals.
src/partials/FeaturedProjectsPartial.vue (1)
6-42: LGTM!Removing the transition wrapper simplifies the component while preserving all rendering states (loading, projects, empty). The change aligns with the PR's goal of simplifying transitions.
src/partials/ArticlesListPartial.vue (1)
24-32: LGTM!The transition removal is clean, and the
aria-busy="true"attribute on the loading state is good for accessibility.tests/partials/ArticleItemPartial.test.ts (1)
3-22: LGTM!The Pinia initialization and API store mocking follows Vue testing best practices, properly setting up the store before each test and providing the necessary searchTerm property.
src/pages/ProjectsPage.vue (1)
27-44: LGTM!The margin adjustment improves spacing, and the transition removal follows the consistent pattern across this PR while preserving all rendering states.
tests/pages/TagPostsPage.test.ts (3)
44-56: LGTM!The reactive store state with getter/setter pattern allows tests to properly simulate search term changes and trigger component watchers. This is a clean and effective testing approach.
126-161: LGTM!Tests correctly updated to reflect the new getPosts signature with text parameter and the unified summary element for all states (loaded, empty, error). The assertions properly verify the integration.
179-194: LGTM!The new test properly verifies that the component refetches posts when the search term changes, passing the updated text parameter to getPosts and updating the summary. This provides good coverage of the search integration feature.
eslint.config.js (1)
186-189: No action required—HTML sanitization is already implemented.ArticleItemPartial.vue properly sanitizes all HTML content before rendering. The component imports DOMPurify (line 68) and the
highlightText()function sanitizes bothitem.titleanditem.excerptusingDOMPurify.sanitize()before they are rendered viav-html(lines 42, 47, 93, 102). No XSS vulnerability exists.tests/partials/HeaderPartial.test.ts (1)
13-24: Router-aware mount helper looks correct and isolates router per test
routesandmountWithRoutercorrectly create a memory router, push the initial/route, awaitisReady, and install the router as a plugin before mounting. ThebeforeEachthat clears mocks keeps tests isolated. Nothing blocking here.Also applies to: 27-29
tests/support/tags.test.ts (1)
1-1: Tag summary tests align well with the new structured APIThe
Tags.summaryFortests now accurately reflect the newTagSummaryDescriptionshape:
- They assert the text/label/suffix combinations for loading, error, empty, and count-based states (including singular vs plural).
- Including
onLabelClick: undefinedin expectations validates the default shape without a callback.- The new callback test correctly checks that
summary.onLabelClickis defined when a handler is passed and that it receives the formatted label'#VUE'.This gives good confidence around the summary behavior.
Also applies to: 53-65, 67-82, 84-98, 100-113
src/pages/TagPostsPage.vue (2)
18-28: New header, summary, and list rendering are cohesive and accessible
- The header row with the “Topics & Tags Explorer” title and “Go back” button is clear and keeps actions discoverable.
- The summary paragraph driven by
summaryContentcleanly handles both labeled and unlabeled states, including suffixes and the clickable label that uses the summary’sonLabelClickhandler.- The articles section’s
v-if="isLoading"skeleton vsv-elselist (with test IDs) nicely separates loading from loaded states without duplicating error/empty branches, since those are now conveyed via the summary.No functional issues spotted here.
Also applies to: 32-52, 55-62
91-92: Tag loading pipeline and summaryContent wiring look correct
- Using
useRoute+Tags.normalizeParam(route.params.tag)fornormalizedTagandTags.formatLabelforformattedTagLabelkeeps tag handling consistent with the shared helper.summaryContentcorrectly delegates toTags.summaryForwith the current tag, loading/error flags, and the post count, and wiresonSummaryLabelClickas the optional label-click callback.loadPostsForTagpreserves the existing requestId-based deduplication and addstext: apiStore.searchTerm.trim()to the API request, which is a straightforward way to couple tag and free-text filters without changing the method signature.This flow should give deterministic results even under rapid tag or search changes.
Also applies to: 106-107, 111-111, 119-121, 149-159, 185-185, 196-205
Moves the logic for setting the search term from to . When navigating to a tag page, the component is now responsible for updating the global search term based on the tag parameter from the URL. This change improves separation of concerns and makes each component more self-contained. The test suite has been updated to reflect these changes: - The test for setting the search term on tag click has been removed from . - Tests in have been updated to assert the new behavior where the component sets the search term on mount and route changes.
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".
| // Escape special regex characters in the search term | ||
| const escapedTerm = term.replace(/[.*+?^${}()|[\\]/g, '\\$&'); | ||
| const regex = new RegExp(`(${escapedTerm})`, 'gi'); |
There was a problem hiding this comment.
The new useTextHighlight helper attempts to escape special characters before building a regular expression, but the literal /[.*+?^${}()|[\\]/g contains an unescaped [ and causes an “Unterminated character class” syntax error as soon as the module is parsed. This will fail the build and prevent any component importing useTextHighlight from rendering. Use a properly escaped character class such as /[.*+?^${}()|[\]\\]/g (or reuse a tested escape helper) so the module can compile.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/setup.ts (1)
21-43: LocalStorageMock is missing Storage interface members.The
LocalStorageMockclass implements core methods but is missing thelengthproperty andkey(index)method required by theStorageinterface. While these are rarely used in tests, the type cast at Line 42 could cause runtime errors if any test accesses these members.Consider completing the interface:
class LocalStorageMock { private store: Record<string, string> = {}; + get length() { + return Object.keys(this.store).length; + } clear() { this.store = {}; } getItem(key: string) { return this.store[key] ?? null; } setItem(key: string, value: string) { this.store[key] = value; } removeItem(key: string) { delete this.store[key]; } + key(index: number) { + const keys = Object.keys(this.store); + return keys[index] ?? null; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
.dockerignore(1 hunks)docker-compose.yml(3 hunks)src/pages/PostPage.vue(1 hunks)src/pages/TagPostsPage.vue(7 hunks)src/partials/ArticleItemPartial.vue(2 hunks)src/partials/ArticlesListPartial.vue(3 hunks)src/partials/HeaderPartial.vue(3 hunks)src/public.ts(2 hunks)src/support/useTextHighlight.ts(1 hunks)tests/pages/AboutPage.test.ts(3 hunks)tests/pages/PostPage.test.ts(8 hunks)tests/pages/TagPostsPage.test.ts(6 hunks)tests/partials/HeaderPartial.test.ts(2 hunks)tests/partials/SideNavPartial.test.ts(2 hunks)tests/setup.ts(1 hunks)tests/support/router.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/support/useTextHighlight.ts (1)
src/stores/api/store.ts (1)
useApiStore(28-133)
tests/partials/HeaderPartial.test.ts (1)
src/stores/api/store.ts (1)
setSearchTerm(34-36)
tests/pages/TagPostsPage.test.ts (2)
src/stores/api/store.ts (1)
getPosts(114-122)src/stores/api/http-error.ts (1)
debugError(29-36)
tests/pages/AboutPage.test.ts (1)
src/stores/api/store.ts (1)
getProfile(42-50)
tests/pages/PostPage.test.ts (1)
src/support/markdown.ts (1)
renderMarkdown(70-78)
🪛 ast-grep (0.40.0)
src/support/useTextHighlight.ts
[warning] 21-21: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((${escapedTerm}), 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (33)
tests/setup.ts (2)
7-12: LGTM! Directive mock is appropriate for test environment.The v-lazy-link directive mock provides empty lifecycle hooks, which is sufficient for tests that need the directive to exist without triggering actual lazy-loading behavior.
14-19: LGTM! Web Crypto polyfill is correctly implemented.The conditional check ensures the polyfill only applies when needed, and using Node's standards-compliant
webcryptois the right approach for jsdom environments.docker-compose.yml (2)
80-80: LGTM! Named volume prevents node_modules conflicts.Adding a named volume for
node_modulesis a best practice that prevents the container from overwriting or conflicting with the host'snode_modulesdirectory, improving performance and consistency in local development.Also applies to: 101-101
62-62: Port change verified and correctly configured.The relay upstream port 18080 is consistently configured across docker-compose.yml and the Caddy WebCaddyfile.local, with no orphaned 8080 references remaining. The change aligns with the local development setup as expected.
tests/partials/SideNavPartial.test.ts (2)
58-65: LGTM! RouterView stub aligns with router-based mounting.Adding the RouterView stub in global mount options is consistent with the broader test environment changes enabling router-aware component mounting.
93-100: LGTM! Test correctly reflects social links removal.The test name and assertion have been properly updated to verify that the social links section no longer renders, aligning with the UI changes in this PR.
src/pages/PostPage.vue (3)
30-32: LGTM! Simplified conditional rendering.Removing the transition wrapper and simplifying to direct conditional rendering (skeleton when loading, post when loaded) improves code clarity and maintainability.
33-114: Post header restructured with improved layout.The reorganization consolidates date and sharing actions in the header, followed by title and tags. This provides a clearer visual hierarchy and better separation of concerns.
105-107: Tag navigation now router-driven.Tags now use
RouterLinkwithTags.routeFor(tag.name)instead of manual click handlers. This approach is more declarative and aligns with Vue Router best practices..dockerignore (1)
1-51: LGTM! Well-organized ignore rules.The reorganization into logical sections (Git, Docker, Node, Editor/IDE, OS, Project-specific, Other, Config files) significantly improves readability and makes the file easier to maintain.
src/public.ts (2)
56-68: LGTM! SSR-aware navigation utility.The
goBackfunction properly handles different environments:
- SSR (no window): navigates to Home
- Client with history: uses router.back()
- Client without history: navigates to Home as fallback
The type-only import at Line 1 is correct.
1-1: Fix the Router import - must be type-only.The
Routerimport at Line 1 is imported as a value, butvue-routeronly exportsRouteras a TypeScript type. This will cause a build error:The requested module 'vue-router' does not provide an export named 'Router'.Apply this diff to fix the import:
-import type { Router as VueRouter } from 'vue-router'; +import type { Router as VueRouter } from 'vue-router';Wait, that's already correct! Let me re-check...
Actually, looking at Line 1, it already uses
import type, so this is correct. The past review comment appears to be outdated or incorrect. The current code is:import type { Router as VueRouter } from 'vue-router';This is the correct syntax for a type-only import.
src/partials/ArticleItemPartial.vue (2)
70-72: LGTM! Using the extracted composable as suggested.The highlighting logic has been properly extracted into the
useTextHighlightcomposable, addressing the code duplication concern from the previous review.
42-42: No issues found — DOMPurify sanitization is properly implemented in both code paths.Verification confirms the
highlight()function sanitizes the content before returning it tov-htmlin both scenarios:
- When no search term exists:
DOMPurify.sanitize(text)at line 17- When search term exists:
DOMPurify.sanitize(highlighted)at line 26 (after mark tags are added)The sanitization flow is correct—the function applies regex-based mark tag insertion to the untrusted text, then sanitizes the complete result before returning. This prevents any malicious HTML or scripts in the input from reaching the DOM.
tests/pages/AboutPage.test.ts (2)
32-59: LGTM! Router-based test mounting improves accuracy.The new
mountComponenthelper properly creates a memory router, navigates to the route, and mounts the component within an App wrapper. This approach more accurately reflects how the component renders in production.
66-72: LGTM! Tests correctly updated for async mounting.All test cases have been properly updated to await the async
mountComponenthelper and verify behavior within the router context.Also applies to: 74-82, 84-91
src/support/useTextHighlight.ts (1)
9-27: Highlight function logic is sound once the escape pattern is fixed.The function correctly handles edge cases (null/undefined text, empty search terms) and applies DOMPurify sanitization. Once the regex escape pattern on Line 21 is corrected, the ReDoS concern flagged by static analysis will be properly mitigated.
tests/support/router.ts (1)
4-9: LGTM!Clean test helper that provides a consistent router setup for tests with memory history and a sensible default route.
src/partials/HeaderPartial.vue (2)
68-76: Good reactive synchronization between store and local state.The watch on
apiStore.searchTerm(lines 71-76) combined with initialization from the store (line 68) ensures that changes to the search term from other components (like TagPostsPage) are automatically reflected in the header's input field. This eliminates the need for DOM manipulation that was flagged in the previous review.Based on learnings
105-108: Verify navigation behavior on search.Every search now navigates to the TagPosts page using the search term as a tag parameter. This means users will be redirected away from their current page (e.g., AboutPage, PostPage) when searching. Confirm this is the intended UX.
src/partials/ArticlesListPartial.vue (3)
25-31: Simplified rendering by removing transition wrapper.Direct conditional rendering is clearer and performs well for this use case. The skeleton/list/empty states are still properly handled.
55-63: Category preservation logic improves UX.The
categoryBeforeSearchstate and updatedselectCategorylogic preserve the user's category choice during search, addressing the concern from the previous review about unexpected category changes.Based on learnings
132-148: Search and category interaction improved.The enhanced watcher now:
- Saves the current category before starting a search (lines 136-139)
- Restores it when the search is cleared (lines 140-143)
- Defaults to the first category only when appropriate (lines 146-148)
This addresses the UX concern from the previous review about unexpected category changes during search/clear operations.
Based on learnings
tests/partials/HeaderPartial.test.ts (1)
12-28: Router integration properly implemented in tests.The
mountWithRouterhelper (lines 17-23) provides consistent router context, and thebeforeEachhook (lines 26-28) ensures clean test isolation. Tests are appropriately async to await router readiness.tests/pages/PostPage.test.ts (1)
80-109: Router-based test mounting implemented correctly.The App wrapper with RouterView (lines 80-83) and async mounting flow (lines 87-109) properly simulate route-driven behavior. The route is correctly configured with path
/posts/:slugand pre-populated with the post slug.tests/pages/TagPostsPage.test.ts (3)
45-56: Reactive store mock enables search term testing.The
storeStatereactive object with getter/setter forsearchTerm(lines 50-55) allows tests to simulate store updates and verify component reactivity to search term changes.
131-131: Verify API text parameter format.Tests expect the
textparameter to include:
- Formatted tag labels with hash prefix (e.g.,
'#DESIGN','#UX') when navigating via tags- Raw search terms without formatting (e.g.,
'new search') when searchingEnsure the backend API correctly handles both formats and that this matches the actual implementation in
TagPostsPage.vue.Also applies to: 169-169, 186-186
174-189: Search term reactivity test provides good coverage.The new test verifies that external search term updates (e.g., from the header) trigger a refetch with the updated text parameter, ensuring proper integration between components via the shared store.
src/pages/TagPostsPage.vue (5)
122-130: Back navigation properly handles search state.The
handleGoBacklogic correctly clears the search term (staying on the page to show all tag posts) when a search is active, or navigates back when no search is present. This provides intuitive UX.
132-135: Label click handler correctly updates search term.Extracting the search term by removing the hash prefix and lowercasing aligns with the expected search term format used throughout the application.
137-147: Summary content composition is well-structured.Using
Tags.summaryForwith the callback pattern provides flexible, testable summary rendering while keeping the component logic clean.
189-192: API call includes search term for refined filtering.The
loadPostsForTagfunction now includesapiStore.searchTermin the request (line 191), enabling combined tag + text filtering. This replaces the previous DOM manipulation approach and uses reactive store state instead.Verify that the backend API correctly handles requests with both
tagandtextparameters, and that the interaction between these filters matches the intended behavior (e.g., AND vs OR logic).Based on learnings
217-238: Watcher coordination is properly implemented—no duplicate API calls occur.Verification confirms the mechanisms work as intended:
Guard prevents duplicate calls: When the tag watcher updates
searchTerm(line 222), the second watcher triggers but the guard condition (line 234) evaluates tofalsebecause bothnewSearchTermandcurrentTagLabelare now equal, soloadPostsForTagis not called a second time.lastRequestId prevents stale responses: Each API call increments
requestId(line 174), and stale responses are discarded before updating state (lines 194, 202, 209). This acts as race condition protection even across different navigation scenarios.Single entry points verified:
loadPostsForTagis called only at lines 224 and 235, both properly coordinated through the guard condition.
Summary by CodeRabbit
New Features
Bug Fixes
Style