-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: No individual data loading indicators when displaying multiple info panels #3026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: No individual data loading indicators when displaying multiple info panels #3026
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
📝 WalkthroughWalkthroughAdded per-object loading state tracking in DataBrowser's multi-panel mode using a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
# [8.1.0-alpha.6](8.1.0-alpha.5...8.1.0-alpha.6) (2025-11-27) ### Bug Fixes * No individual data loading indicators when displaying multiple info panels ([#3026](#3026)) ([5ca8218](5ca8218))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/dashboard/Data/Browser/DataBrowser.react.js (1)
106-137: Per-object loading tracking withloadingObjectIdsis sound; minor readability tweak possibleThe new
loadingObjectIdsSet and its use infetchDataForMultiPanellook correct:
- You’re using functional
setStateand cloning the Set (new Set(prev.loadingObjectIds)) before mutating, so you avoid mutating state in place.- Both the success and error paths reliably remove the
objectId, so individual panels shouldn’t get stuck in a loading state.- Success path also updates
prefetchCacheandmultiPanelData, which keeps multi-panel rendering in sync with the new loading tracking.One small readability improvement you might consider is avoiding the inline mutation:
this.setState(prev => ({ loadingObjectIds: new Set(prev.loadingObjectIds).add(objectId) }));and mirroring the pattern you already use in the success/error handlers:
this.setState(prev => { const loadingObjectIds = new Set(prev.loadingObjectIds); loadingObjectIds.add(objectId); return { loadingObjectIds }; });Functionally it’s the same, but it’s a bit clearer to future readers.
Also applies to: 981-1044
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/dashboard/Data/Browser/DataBrowser.react.js(3 hunks)src/dashboard/Data/Browser/Databrowser.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-31T06:12:17.707Z
Learnt from: mtrezza
Repo: parse-community/parse-dashboard PR: 2957
File: src/dashboard/Data/Browser/BrowserTable.react.js:584-597
Timestamp: 2025-07-31T06:12:17.707Z
Learning: In Parse Dashboard's data browser selection logic (src/dashboard/Data/Browser/BrowserTable.react.js), the `selection['*']` pattern is used to handle global operations that pass `{ '*': true }` to indicate all items are selected, particularly for bulk operations like delete. This is not dead code but serves as compatibility layer for global operations that don't go through normal individual row selection workflows.
Applied to files:
src/dashboard/Data/Browser/DataBrowser.react.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (2)
src/dashboard/Data/Browser/Databrowser.scss (1)
34-66: Per-panel stacking context viaposition: relativelooks appropriateMaking
.panelColumnpositioned is a good way to scope any absolutely positioned loading overlays to each column, and it shouldn’t interfere with the existing scroll container or the sticky header in.panelHeader. No issues spotted here.src/dashboard/Data/Browser/DataBrowser.react.js (1)
1476-1533: Per-panelisLoadingcomputation aligns with multi-panel UX goalsThe updated
isLoadingexpression:const isLoading = (objectId === this.state.selectedObjectId && this.props.isLoadingCloudFunction) || this.state.loadingObjectIds.has(objectId);correctly:
- Preserves existing behavior for the currently selected row (still driven by
isLoadingCloudFunction).- Adds granular per-object loading state via
loadingObjectIdsso that only panels with in-flightfetchDataForMultiPanelcalls show a loading indicator.This should resolve the “one big loader over all panels” problem while keeping single-panel mode unchanged. Implementation here looks good.
|
🎉 This change has been released in version 8.1.0-alpha.6 |
New Pull Request Checklist
Issue Description
There are no individual loading indicators when displaying multiple info panels. Instead there is only 1 main loading indicator that overlays the whole sidebar.
Approach
Add individual loading indicators.
Summary by CodeRabbit
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.