Skip to content

Conversation

@richardfogaca
Copy link
Contributor

SUMMARY

Fixes issue where the "Save" button in dashboard edit mode was incorrectly enabled even when no changes were made. This occurred because the theme change detection effect was running on every render, treating the existing theme value as a "change" and marking the dashboard as having unsaved modifications.

Changes:

  • Added previousThemeRef to track the actual previous theme value
  • Modified theme change detection to compare against the previous value instead of just checking if theme exists
  • Only mark as unsaved when theme actually changes (not on initial render or navigation)
  • Reset the theme ref when exiting edit mode to prevent false positives when navigating between dashboards
  • Clear unsaved changes flag when entering edit mode to ensure clean state

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

  • Save button now disabled when clicking on "Edit dashboard" (on initial page load):
Screenshot 2025-10-23 at 08 06 53

TESTING INSTRUCTIONS

  1. Open a dashboard and enter edit mode
  2. Verify the "Save" button is disabled (no changes made yet)
  3. Change the dashboard theme
  4. Verify "Save" button is now enabled
  5. Save the changes and exit edit mode
  6. Navigate to another dashboard and enter edit mode
  7. Verify "Save" button is disabled (should not show false positive)

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Oct 23, 2025

Code Review Agent Run #bfddc5

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 2171a30..2171a30
    • superset-frontend/src/dashboard/components/Header/index.jsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot bot added change:frontend Requires changing the frontend dashboard:editmode Related to te Dashboard edit mode labels Oct 23, 2025
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've completed my review and didn't find any issues.

Files scanned
File Path Reviewed
superset-frontend/src/dashboard/components/Header/index.jsx

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@bito-code-review
Copy link
Contributor

Interaction Diagram by Bito
sequenceDiagram
participant USER as User
participant HDR as Dashboard Header<br/>🔄 Updated | ●●○ Medium
participant REF as previousThemeRef<br/>🟩 Added | ●○○ Low
participant EFFECT as useEffect Hook<br/>🔄 Updated | ●●○ Medium
participant ACT as boundActionCreators
participant STORE as Redux Store
participant STATE as dashboardState Reducer
Note over HDR, REF: Theme tracking enhancement<br/>prevents false positives
USER->>HDR: Navigate to dashboard
HDR->>REF: Initialize with dashboardInfo.theme
HDR->>EFFECT: Monitor theme & editMode changes
EFFECT->>REF: Compare current vs previous theme
alt Edit Mode & Theme Changed
EFFECT->>ACT: setUnsavedChanges(true)
ACT->>STORE: Dispatch SET_UNSAVED_CHANGES
STORE->>STATE: Update hasUnsavedChanges
EFFECT->>REF: Update previousThemeRef
else Not Edit Mode
EFFECT->>REF: Sync previousThemeRef
    end
USER->>HDR: Click Edit Dashboard button
HDR->>ACT: setUnsavedChanges(false)
ACT->>STORE: Reset unsaved changes flag
Loading

Critical path: User->Dashboard Header->useEffect Hook->boundActionCreators->Redux Store

Note: The Header component now uses a ref to track previous theme values, preventing false positive unsaved change notifications when navigating between dashboards. The edit button click also explicitly resets the unsaved changes flag to ensure clean state initialization.

@rusackas rusackas requested a review from msyavuz October 27, 2025 17:27
Copy link
Member

@msyavuz msyavuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, this looks testable, can we get some tests?

@EnxDev EnxDev added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Nov 13, 2025
@github-actions github-actions bot added 🎪 5e64f4a 🚦 building Environment 5e64f4a status: building 🎪 5e64f4a 📅 2025-11-13T12-04 Environment 5e64f4a created at 2025-11-13T12-04 🎪 5e64f4a ⌛ 48h Environment 5e64f4a expires after 48h and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Nov 13, 2025
@github-actions
Copy link
Contributor

🎪 Showtime is building environment on GHA for 5e64f4a

@github-actions github-actions bot added 🎪 5e64f4a 🤡 EnxDev Environment 5e64f4a requested by EnxDev 🎪 5e64f4a 🚦 deploying Environment 5e64f4a status: deploying 🎪 5e64f4a 🚦 running Environment 5e64f4a status: running 🎪 🎯 5e64f4a Active environment pointer - 5e64f4a is receiving traffic 🎪 5e64f4a 🌐 34.215.78.155:8080 Environment 5e64f4a URL: http://34.215.78.155:8080 (click to visit) and removed 🎪 5e64f4a 🚦 building Environment 5e64f4a status: building 🎪 5e64f4a 🚦 deploying Environment 5e64f4a status: deploying 🎪 5e64f4a 🚦 running Environment 5e64f4a status: running 🎪 🎯 5e64f4a Active environment pointer - 5e64f4a is receiving traffic labels Nov 13, 2025
@github-actions
Copy link
Contributor

🎪 Showtime deployed environment on GHA for 5e64f4a

Environment: http://34.215.78.155:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

Copy link
Contributor

@EnxDev EnxDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! We just need to address this comment

@richardfogaca
Copy link
Contributor Author

Tests added, thanks for the heads up @msyavuz @EnxDev :)

Copy link
Contributor

@EnxDev EnxDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@EnxDev EnxDev merged commit 467b008 into apache:master Nov 13, 2025
70 of 71 checks passed
@sadpandajoe sadpandajoe added the v6.0 Label added by the release manager to track PRs to be included in the 6.0 branch label Nov 14, 2025
aminghadersohi pushed a commit to aminghadersohi/superset that referenced this pull request Nov 17, 2025
kshi020302 pushed a commit to jl141/superset that referenced this pull request Nov 30, 2025
Facyla pushed a commit to Facyla/superset-contrib that referenced this pull request Dec 16, 2025
@github-actions github-actions bot added 🍒 6.0.0 Cherry-picked to 6.0.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels change:frontend Requires changing the frontend dashboard:editmode Related to te Dashboard edit mode 🎪 🔒 showtime-blocked size/L v6.0 Label added by the release manager to track PRs to be included in the 6.0 branch 🎪 5e64f4a 🤡 EnxDev Environment 5e64f4a requested by EnxDev 🎪 5e64f4a 🚦 running Environment 5e64f4a status: running 🎪 5e64f4a 🌐 34.215.78.155:8080 Environment 5e64f4a URL: http://34.215.78.155:8080 (click to visit) 🎪 5e64f4a ⌛ 48h Environment 5e64f4a expires after 48h 🎪 5e64f4a 📅 2025-11-13T12-04 Environment 5e64f4a created at 2025-11-13T12-04 🍒 6.0.0 Cherry-picked to 6.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants