Skip to content

Conversation

@mtrezza
Copy link
Member

@mtrezza mtrezza commented Nov 27, 2025

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

    • Enhanced loading state tracking for multi-panel data fetching in the data browser.
  • Style

    • Updated column layout positioning in the data browser.

✏️ Tip: You can customize this high-level summary in your review settings.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Nov 27, 2025

🚀 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.

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

📝 Walkthrough

Walkthrough

Added per-object loading state tracking in DataBrowser's multi-panel mode using a loadingObjectIds Set. This Set is updated when fetching multi-panel data—objects are added before async calls and removed on completion—enabling granular loading indicators. Additionally, applied a layout positioning adjustment to panelColumn.

Changes

Cohort / File(s) Summary
Multi-panel loading state tracking
src/dashboard/Data/Browser/DataBrowser.react.js
Introduced loadingObjectIds Set to state for tracking in-progress fetches per object. Updated multi-panel fetch logic to add/remove objectIds from the Set on request start, success, and error; adjusted rendering to treat objects as loading if their id is in the Set or if isLoadingCloudFunction is true.
Layout adjustment
src/dashboard/Data/Browser/Databrowser.scss
Added position: relative; to .panelColumn to establish a new stacking context and alter layout behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus areas:
    • Verify loadingObjectIds is properly initialized and cleared in all async code paths (success/error handlers) to prevent memory leaks or stale state
    • Confirm render logic correctly checks both loadingObjectIds and isLoadingCloudFunction conditions without redundancy
    • Validate that the position: relative; CSS change doesn't introduce unintended layout regressions, particularly with scroll or positioning of child elements

Possibly related PRs

  • #3020 — Modifies DataBrowser.react.js for multi-panel data fetching and per-panel state management, sharing similar refetch and caching patterns.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the issue and approach, but lacks required details like closed issue reference and doesn't address the TODOs checklist section. Complete the 'Closes: FILL_THIS_OUT' field with the issue number and clarify status of tests and documentation updates needed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: adding individual loading indicators for multiple info panels instead of a single overlay indicator.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@parseplatformorg
Copy link
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mtrezza mtrezza changed the title fix: No individual loading indicators when displaying multiple info panels fix: No individual data loading indicators when displaying multiple info panels Nov 27, 2025
@mtrezza mtrezza merged commit 5ca8218 into parse-community:alpha Nov 27, 2025
10 of 11 checks passed
@mtrezza mtrezza deleted the fix/panel-loading-indicator branch November 27, 2025 18:36
parseplatformorg pushed a commit that referenced this pull request Nov 27, 2025
# [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))
Copy link

@coderabbitai coderabbitai bot left a 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 with loadingObjectIds is sound; minor readability tweak possible

The new loadingObjectIds Set and its use in fetchDataForMultiPanel look correct:

  • You’re using functional setState and 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 prefetchCache and multiPanelData, 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

📥 Commits

Reviewing files that changed from the base of the PR and between 7a01664 and 0be7308.

📒 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 via position: relative looks appropriate

Making .panelColumn positioned 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-panel isLoading computation aligns with multi-panel UX goals

The updated isLoading expression:

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 loadingObjectIds so that only panels with in-flight fetchDataForMultiPanel calls 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.

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 8.1.0-alpha.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants