(SP: 2) [Frontend] Q&A Viewed and Bookmark Indicators#402
Merged
ViktorSvertoka merged 2 commits intodevelopfrom Mar 15, 2026
Merged
(SP: 2) [Frontend] Q&A Viewed and Bookmark Indicators#402ViktorSvertoka merged 2 commits intodevelopfrom
ViktorSvertoka merged 2 commits intodevelopfrom
Conversation
ViktorSvertoka
added
enhancement
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds localStorage-backed viewed and bookmarked state to Q&A accordion items, introduces leading/trailing slots on AccordionTrigger, updates UI to show Viewed badge and bookmark control, and includes tests covering persisted state and cross-tab sync. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as "User (click)"
participant Accordion as "AccordionList\n(Component)"
participant Trigger as "AccordionTrigger\n(UI)"
participant Storage as "localStorage"
participant OtherTab as "Other Tab\n(storage event listener)"
rect rgba(200,200,255,0.5)
User->>Trigger: click open
Trigger->>Accordion: onClick -> markAsViewed(questionId)
Accordion->>Storage: read/write viewed set
Storage-->>Accordion: persisted viewed IDs
Accordion-->>Trigger: render Viewed badge + bookmark control
end
rect rgba(200,255,200,0.5)
User->>Trigger: toggle bookmark
Trigger->>Accordion: toggleBookmark(questionId)
Accordion->>Storage: update bookmarked set
Storage-->>OtherTab: storage event
OtherTab->>Accordion: update state from storage event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can get early access to new features in CodeRabbit.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/components/tests/q&a/accordion-list.test.tsx (1)
35-53: Forward all trigger props in the mock to avoid hiding interaction regressions.The mock currently drops non-
onClickprops. Forwarding the rest keeps test behavior closer to production trigger usage.Mock fidelity improvement
AccordionTrigger: ({ children, leading, trailing, - onClick, + ...props }: { children: React.ReactNode; leading?: React.ReactNode; trailing?: React.ReactNode; - onClick?: () => void; - }) => ( + } & React.ButtonHTMLAttributes<HTMLButtonElement>) => ( <div> {leading} - <button type="button" onClick={onClick}> + <button type="button" {...props}> {children} </button> {trailing} </div> ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/tests/q`&a/accordion-list.test.tsx around lines 35 - 53, The mock AccordionTrigger currently destructures only children, leading, trailing, and onClick and drops any other props; update the AccordionTrigger mock to accept a rest props object (e.g., ...props) alongside the named props and spread those props onto the rendered <button> so all incoming trigger props (aria attributes, className, disabled, data-*, etc.) are forwarded; keep the same component name AccordionTrigger and ensure the button still uses the onClick handler and renders leading/children/trailing as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/components/q`&a/AccordionList.tsx:
- Around line 67-70: The writeStoredQuestionIds function calls
window.localStorage.setItem directly which can throw in restricted environments
or when quota is exceeded; update writeStoredQuestionIds to first check typeof
window !== 'undefined' and that window.localStorage is available, then wrap
JSON.stringify([...ids]) and localStorage.setItem(storageKey, ...) in a
try/catch, swallowing or emitting a non-fatal warning (e.g., console.warn) on
error so clicks/bookmarks don’t break; reference the function name
writeStoredQuestionIds and the parameters storageKey and ids when making this
change.
- Around line 372-381: The storage listener in handleStorage currently ignores
clear events (when e.key === null), leaving stale UI; update handleStorage (the
function named handleStorage) so that when e.key === null it performs the same
updates as when CACHE_KEY, QA_VIEWED_STORAGE_KEY, or QA_BOOKMARK_STORAGE_KEY
change: call refreshCachedTerms() and re-read both viewed/bookmarked IDs via
setViewedItems(readStoredQuestionIds(QA_VIEWED_STORAGE_KEY)) and
setBookmarkedItems(readStoredQuestionIds(QA_BOOKMARK_STORAGE_KEY)); keep the
existing per-key branches but add an early branch for e.key === null to sync
cross-tab localStorage.clear() events.
- Around line 495-545: The bookmark control is currently nested inside the
AccordionTrigger which breaks accessibility; move the bookmark span out of the
trigger and render it via the AccordionTrigger/trailing prop so it becomes a
sibling interactive control. Locate the AccordionTrigger usage (the component
wrapping q.question) and remove the nested span with role="button" and its
tabIndex/aria-* props from inside the trigger; instead render a standalone
bookmark control in the trailing prop that uses toggleBookmark(questionId) for
onClick and onKeyDown, applies the same isViewed/isBookmarked-based aria-label
and aria-pressed logic, and preserves the Badge and Bookmark visuals and
focus/hover classes so it remains keyboard-accessible and not nested inside the
trigger.
---
Nitpick comments:
In `@frontend/components/tests/q`&a/accordion-list.test.tsx:
- Around line 35-53: The mock AccordionTrigger currently destructures only
children, leading, trailing, and onClick and drops any other props; update the
AccordionTrigger mock to accept a rest props object (e.g., ...props) alongside
the named props and spread those props onto the rendered <button> so all
incoming trigger props (aria attributes, className, disabled, data-*, etc.) are
forwarded; keep the same component name AccordionTrigger and ensure the button
still uses the onClick handler and renders leading/children/trailing as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3ec166a-1110-47dc-be42-e08bf04b8a84
📒 Files selected for processing (3)
frontend/components/q&a/AccordionList.tsxfrontend/components/tests/q&a/accordion-list.test.tsxfrontend/components/ui/accordion.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Viewedindicator to Q&A accordion items after the user opens a questionCloses #401
Summary by CodeRabbit
New Features
Tests