feat(recents): show progress bar while shared-drive files load#3947
Conversation
Shared-drive and federated files are fetched from the stack via the dataproxy and arrive after the local files, so the recents list silently grows once it already looks settled. Surface that background fetch with a progress bar under the header once a list is on screen.
Walkthrough
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Our agent can fix these. Install it.
Gates Passed
3 Quality Gates Passed
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/modules/views/Recent/index.loading.spec.jsx (2)
76-79: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winRestore spies after each test to prevent mock leakage.
consolespies are created in each test but never restored. Add cleanup for isolation and to avoid cross-test side effects.Suggested patch
describe('Recent View loading indicator', () => { beforeEach(() => { jest.spyOn(console, 'error').mockImplementation() jest.spyOn(console, 'warn').mockImplementation() }) + + afterEach(() => { + jest.restoreAllMocks() + })🤖 Prompt for 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. In `@src/modules/views/Recent/index.loading.spec.jsx` around lines 76 - 79, The console spies created in the beforeEach hook using jest.spyOn for console.error and console.warn are never restored, causing mock leakage between tests. Add an afterEach hook that calls mockRestore() on both spies to restore the original console implementations after each test completes, ensuring proper test isolation.
48-48: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winProvide the full
useSharingContextshape in the mock.
RecentViewreadsallLoaded,refresh, andisOwnerin addition tobyDocId. Stubbing onlybyDocIdmakes this test fragile to harmless component changes.Suggested patch
-useSharingContext.mockReturnValue({ byDocId: [] }) +useSharingContext.mockReturnValue({ + byDocId: [], + allLoaded: true, + isOwner: jest.fn(() => true), + refresh: jest.fn() +})🤖 Prompt for 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. In `@src/modules/views/Recent/index.loading.spec.jsx` at line 48, The useSharingContext mock at the useSharingContext.mockReturnValue() call currently only provides the byDocId property, but the RecentView component also reads allLoaded, refresh, and isOwner from this context. Update the mock return object to include all four properties (byDocId, allLoaded, refresh, and isOwner) with appropriate stub values to ensure the test properly reflects what the component actually uses and prevents the test from becoming fragile to harmless component changes.
🤖 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.
Nitpick comments:
In `@src/modules/views/Recent/index.loading.spec.jsx`:
- Around line 76-79: The console spies created in the beforeEach hook using
jest.spyOn for console.error and console.warn are never restored, causing mock
leakage between tests. Add an afterEach hook that calls mockRestore() on both
spies to restore the original console implementations after each test completes,
ensuring proper test isolation.
- Line 48: The useSharingContext mock at the useSharingContext.mockReturnValue()
call currently only provides the byDocId property, but the RecentView component
also reads allLoaded, refresh, and isOwner from this context. Update the mock
return object to include all four properties (byDocId, allLoaded, refresh, and
isOwner) with appropriate stub values to ensure the test properly reflects what
the component actually uses and prevents the test from becoming fragile to
harmless component changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e56e6be2-e995-4b0a-904f-2eb2de33b87d
📒 Files selected for processing (2)
src/modules/views/Recent/index.jsxsrc/modules/views/Recent/index.loading.spec.jsx
BundleMonFiles updated (1)
Unchanged files (19)
Total files change +40B 0% Groups updated (1)
Unchanged groups (2)
Final result: ✅ View report in BundleMon website ➡️ |
|
let's try it |
Context
In the recents view, shared-drive and federated-share files are fetched from the stack through the dataproxy and arrive a few seconds after the local files (longer for accounts with many drives). Until then the list looks settled, then silently grows with no indication anything was pending.
Solution
Show a thin progress bar under the header while that background fetch is in flight, once a list is already on screen. The empty initial load keeps its existing skeleton.
Summary by CodeRabbit
New Features
Tests