unified saved hub and bookmark refactor (Closes #132) - #138
Merged
Conversation
|
@Ahbiz is attempting to deploy a commit to the Deen Bridge Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a ChangesSaved hub and bookmark behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Learner
participant SavedPage
participant CourseCard
participant LibraryBookCard
participant useBookmarkCore
participant BookmarkActions
Learner->>SavedPage: open Saved route
SavedPage->>BookmarkActions: fetch saved courses and books
BookmarkActions-->>SavedPage: return saved items
SavedPage->>CourseCard: render seeded course bookmarks
SavedPage->>LibraryBookCard: render seeded book bookmarks
Learner->>CourseCard: toggle bookmark
CourseCard->>useBookmarkCore: invoke toggle
useBookmarkCore->>BookmarkActions: toggle course bookmark
BookmarkActions-->>useBookmarkCore: return result
useBookmarkCore-->>SavedPage: report bookmark change
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/dashboard/saved/page.jsx`:
- Around line 55-65: Update handleCourseBookmarkChange and
handleBookBookmarkChange to cache each optimistically removed course or book,
then restore that cached item when the same callback receives isBookmarked=true
after a failed toggle. Preserve filtering on false, avoid duplicate restoration,
and centralize the shared removal/restoration logic so both handlers support
rollback consistently.
In `@hooks/useBookmarkCore.js`:
- Around line 41-63: Guard the asynchronous checkBookmark response against an
intervening toggle by tracking toggle activity with a ref, and ignore any check
result that becomes stale after toggle() starts or completes. Update the
relevant toggle flow in toggle() and the response handling in checkBookmark so
the confirmed toggle state cannot be overwritten by the mount-time request;
preserve the existing optimistic update and error-revert behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dcdc8636-53ef-4f1e-a939-7e51d41dea8a
📒 Files selected for processing (9)
app/dashboard/saved/page.jsxcomponents/atoms/BookmarkButton.jsxcomponents/molecules/dashboard/cards/courseCard.jsxcomponents/molecules/dashboard/cards/libraryCard.jsxcomponents/molecules/dashboard/nav-routers.jsxhooks/useBookBookmark.jshooks/useBookmark.jshooks/useBookmarkCore.jspublic/sw.js
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
2 participants
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.
Summary
Resolves #132 by creating a unified "Saved" Hub route (
/dashboard/saved), consolidating duplicated bookmark hook logic into a shared core, and reducing per-card API check overhead.What Was Built
1. Unified Saved Hub (
/dashboard/saved)/dashboard/savedroute with tabbed navigation ("Saved Courses" and "Saved Books").CourseCardSkeleton,LibraryBookSkeleton), empty state call-to-actions, andNetworkErrorComperror handling.2. Consolidated Bookmark Hook Core (
useBookmarkCore.js)useBookmarkanduseBookBookmarkinto a genericuseBookmarkCorehook.useBookmark.jsanduseBookBookmark.jsto act as thin wrappers arounduseBookmarkCore, keeping all existing exports and signatures 100% backward compatible.initialIsBookmarkedpre-seeding support so components with known bookmark state bypass unnecessary per-card mount check network requests (3. Reusable
BookmarkButtonComponent (components/atoms/BookmarkButton.jsx)BookmarkButtonatom supporting both course and book visual styles.4. Navigation Link
Bookmarkicon to the main dashboard sidebar (components/molecules/dashboard/nav-routers.jsx).Verification
npm run lint— passed cleanly with 0 errors.npm run build— compiled successfully and generated production route/dashboard/saved.Closes #132
Summary by CodeRabbit
New Features
Improvements