feat(ui): Prevent Face Collection from Pushing Tagged Images Below the Fold - #1457
feat(ui): Prevent Face Collection from Pushing Tagged Images Below the Fold#1457akshajtiwari wants to merge 3 commits into
Conversation
Walkthrough
ChangesFace collection pagination
Idle polling test timing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@frontend/src/components/FaceCollections.tsx`:
- Line 51: Replace the any annotations in the sorting and rendering callbacks
within FaceCollections with the existing Cluster type, or remove the annotations
where TypeScript can infer Cluster safely. Ensure both cluster paths remain
explicitly typed without introducing any.
- Around line 125-151: Add accessible names to the pagination buttons in
FaceCollections by adding “Previous page” to the button containing ChevronLeft
and “Next page” to the button containing ChevronRight.
- Around line 23-25: Update the FaceCollections pagination rendering around
PAGE_SIZE and the cluster list so incomplete pages reserve the missing
responsive grid slots, using inert placeholders or an equivalent responsive
minimum-height approach. Ensure every page maintains the same card height from
sm through xl without affecting real cluster interactions.
- Around line 56-60: Synchronize the pagination state with the clamped value: in
the FaceCollections pagination logic around totalPages, update page when
totalPages shrinks so it does not remain out of range. Preserve the existing
currentPage calculation and ensure the previous/next handlers operate from the
synchronized state without requiring extra clicks.
🪄 Autofix
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: ce72251c-820a-4a22-954b-fc4280b2a202
📒 Files selected for processing (2)
frontend/src/components/FaceCollections.tsxfrontend/src/hooks/__tests__/useRefreshMemories.test.tsx
|
@rohan-pandeyy PR is ready for review!! |
Addressed Issues:
Fixes #1420
Screenshots/Recordings:
Before
After
Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Claude
Checklist
Summary by CodeRabbit
New Features
Tests