fix(dashboard): offer Exit edit mode when there is nothing to discard - #42208
Conversation
Follow-up to apache#40832, which disabled the Discard button when a dashboard edit session has no unsaved changes. That fixed the misleading label but left the toolbar with no way back to view mode on a pristine session (Save and Discard both disabled, and the actions menu has no exit item). Keep the button always enabled instead, labeled Discard when there are unsaved changes and Exit edit mode when there are none. The handler is unchanged in both cases: it drops the ?edit param and returns to view mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code Review Agent Run #03edc2Actionable Suggestions - 0Additional Suggestions - 1
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42208 +/- ##
==========================================
- Coverage 65.19% 65.18% -0.01%
==========================================
Files 2767 2767
Lines 156062 156064 +2
Branches 35717 35719 +2
==========================================
- Hits 101741 101737 -4
- Misses 52361 52367 +6
Partials 1960 1960
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
🎪 Showtime deployed environment on GHA for f898981 • Environment: http://34.220.202.197:8080 (admin/admin) |
There was a problem hiding this comment.
Pull request overview
This PR removes the “dead end” in dashboard edit mode when there are no unsaved changes by keeping the secondary action button enabled and making its label accurately reflect what will happen when clicked (exit edit mode vs discard changes).
Changes:
- Keep the secondary button enabled in edit mode regardless of
hasUnsavedChanges. - Swap the button’s accessible name and visible label between Discard (when there are unsaved changes) and Exit edit mode (when there are none).
- Update unit tests to reflect the new conditional labeling/enablement behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| superset-frontend/src/dashboard/components/Header/index.tsx | Makes the “Discard” button always clickable and conditionally labels it “Exit edit mode” when there’s nothing to discard. |
| superset-frontend/src/dashboard/components/Header/Header.test.tsx | Updates expectations to assert the correct conditional label and enabled state for the button. |
|
🎪 Showtime deployed environment on GHA for f898981 • Environment: http://35.94.12.182:8080 (admin/admin) |
…apache#42208) Co-authored-by: Claude Code <noreply@anthropic.com>
* refactor(mcp): dedupe list-tool schemas and delete dead middleware (apache#41923) * fix(ag-grid-table): respect row limit with server pagination (apache#41346) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(table/pivot-table): correct non-additive totals/subtotals via DB rollup [SIP-216] (apache#41184) Co-authored-by: Superset Dev <dev@superset.apache.org> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com> * feat(datasets): add RLS filter indicator badge to dataset list and explore view (apache#38807) Co-authored-by: Evan <evan@preset.io> * fix(chart): updates counties of kenya map (apache#38019) Co-authored-by: Zack Adams <zack@Zacks-Laptop.local> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Evan <evan@preset.io> Co-authored-by: Evan Rusackas <evan@rusackas.com> * chore(deps): bump actions/setup-go from 6.5.0 to 7.0.0 (apache#42303) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump nanoid from 5.0.9 to 6.0.0 in /superset-frontend (apache#42230) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: hainenber <dotronghai96@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: hainenber <dotronghai96@gmail.com> Co-authored-by: Joe Li <joe@preset.io> Co-authored-by: Evan <evan@preset.io> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * test(dashboard): migrate dashboard load smoke test to Playwright (apache#41432) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(superset_app_root): when used with oauth (apache#38033) Signed-off-by: Grégoire Bellon-Gervais <gregoire.bellon-gervais@docaposte.fr> Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * refactor: engine enforce SQLAlchemy 2.0 (apache#42277) * fix(ag-grid-table): avoid ambiguous build query import (apache#42313) * fix: Revert "chore(deps): bump echarts from 5.6.0 to 6.1.0 in /superset-frontend" (apache#42314) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * adding circleci config * fix(plugin-chart-echarts): import the -obj locale build so time axes render (apache#42317) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(pandas_postprocessing): avoid FutureWarning for max/min in boxplot MINMAX (apache#42272) * fix(heatmap): correct tooltip axis value lookup and percentage calculations and add tests (apache#41864) Signed-off-by: yousoph <sophieyou12@gmail.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix(forecast): resolve time grain robustly for Prophet forecasting (apache#42145) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix: Added PostgreSQL 17.X to the supported database versions table in (apache#42280) * fix(helm): add MCP HTTPRoute configuration (apache#42219) * fix(dashboard): offer Exit edit mode when there is nothing to discard (apache#42208) Co-authored-by: Claude Code <noreply@anthropic.com> * chore(deps): bump actions/labeler from 6.2.0 to 7.0.0 (apache#42332) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump github/codeql-action/analyze from 4.37.0 to 4.37.1 (apache#42331) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump github/codeql-action/init from 4.37.0 to 4.37.1 (apache#42334) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump @formatjs/intl-durationformat from 0.10.17 to 0.10.18 in /superset-frontend (apache#42337) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump caniuse-lite from 1.0.30001805 to 1.0.30001806 in /docs (apache#42333) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump echarts to 6.1.0 with locale and containLabel guards (apache#42315) (apache#42321) Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com> * docs: add pattern to the list of organisations using superset (apache#42341) * chore(deps): bump ag-grid from 36.0.0 to 36.0.1 in /superset-frontend (apache#42338) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: hainenber <dotronghai96@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: hainenber <dotronghai96@gmail.com> * fix(mcp): trust dataset is_dttm flag when applying time_grain to VARCHAR temporal columns (apache#42288) * chore(deps-dev): update taos-ws-py requirement from >=0.6.9 to >=0.7.0 (apache#42344) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump the storybook group in /superset-frontend with 5 updates (apache#42355) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix(native-filters): keep filter value input caret at inline start (apache#42323) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(mcp): truncate query-tool responses instead of hard-failing (apache#42244) * chore(deps): bump nh3 from 0.3.5 to 0.3.6 (apache#42349) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump pydantic from 2.11.7 to 2.13.4 (apache#42350) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump sqlalchemy-continuum from 1.6.0 to 1.7.0 (apache#42351) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * fix(embedded): stop rejecting guest chart data built from control-specific params keys (apache#42295) Co-authored-by: Claude Code <noreply@anthropic.com> * fix(explore): show the beginning date on time-series x-axis line charts (apache#42046) * fix(api): add example to get_export_ids_schema so Swagger "Try it out" pre-fills a valid array (apache#42265) * chore: SQLAlchemy User cascade backref warnings are irrelevant (apache#42360) * fix(charts): handle async (202) chart-data responses in StatefulChart (apache#42157) Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Evan Rusackas <evan@preset.io> * feat(KustoKQL): Add support for NULL / IS NOT NULL operator (apache#37890) Co-authored-by: ag-ramachandran <ramacg@microsoft.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump pillow from 12.2.0 to 12.3.0 (apache#42348) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump flask-compress from 1.17 to 1.24 (apache#42346) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps-dev): bump databricks-sql-connector from 4.2.6 to 4.3.0 (apache#42347) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> Co-authored-by: Evan Rusackas <evan@preset.io> * fix(native-filters): use FILTER_STATE_CACHE_CONFIG timeout for dynamic filter option queries (apache#38910) * fix(explore): render Jinja before validating legacy chart filters (apache#41996) * fix(dataset): disable duplicate button when name is empty (apache#42217) Co-authored-by: AS-MAC-1123 <as-mac-1123@AS-MAC-1123.local> Co-authored-by: Evan Rusackas <evan@rusackas.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * docs(gunicorn): correct dead links in values.yaml (apache#42385) * chore(deps): bump @deck.gl/mapbox from 9.3.6 to 9.3.7 in /superset-frontend in the deckgl group (apache#42377) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump actions/checkout from 7.0.0 to 7.0.1 (apache#42376) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump immer from 11.1.11 to 11.1.15 in /superset-frontend (apache#42378) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump body-parser from 1.20.5 to 1.20.6 in /docs (apache#42370) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump brace-expansion from 1.1.15 to 1.1.16 in /superset-embedded-sdk (apache#42369) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix(security): bump pyasn1 from 0.6.3 to 0.6.4 (apache#42363) * fix(security): bump pillow from 12.2.0 to 12.3.0 (apache#42362) * docs(map-tiles): add Yandex Maps Tiles API configuration (apache#42375) * ci: improve conditional checks for lillio tests and build --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: hainenber <dotronghai96@gmail.com> Signed-off-by: Grégoire Bellon-Gervais <gregoire.bellon-gervais@docaposte.fr> Signed-off-by: yousoph <sophieyou12@gmail.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com> Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Superset Dev <dev@superset.apache.org> Co-authored-by: SkinnyPigeon <e.blackledge@stuart.com> Co-authored-by: Zack <adams.z.d@gmail.com> Co-authored-by: Zack Adams <zack@Zacks-Laptop.local> Co-authored-by: Evan Rusackas <evan@rusackas.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: hainenber <dotronghai96@gmail.com> Co-authored-by: Joe Li <joe@preset.io> Co-authored-by: Grégoire <greggbg@gmail.com> Co-authored-by: Hans Yu <hans.yu@outlook.de> Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> Co-authored-by: yousoph <sophieyou12@gmail.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Amitesh Gupta <143833521+singlaamitesh@users.noreply.github.com> Co-authored-by: David <39565245+dmunozv04@users.noreply.github.com> Co-authored-by: Yash Shrivastava <119301033+alephys26@users.noreply.github.com> Co-authored-by: JUST.in DO IT <justin.park@airbnb.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: jesperct <jmilecelento@gmail.com> Co-authored-by: Abdul Rehman <76230556+Abdulrehman-PIAIC80387@users.noreply.github.com> Co-authored-by: jenwitteng <jenwit.amonpongitsara@agoda.com> Co-authored-by: Ramachandran A G <106139410+ag-ramachandran@users.noreply.github.com> Co-authored-by: ag-ramachandran <ramacg@microsoft.com> Co-authored-by: Ujjwal Jain <jainujjwal1609@gmail.com> Co-authored-by: Jean Massucatto <massucattoj@gmail.com> Co-authored-by: suvankardas216 <rohanrohan510@gmail.com> Co-authored-by: AS-MAC-1123 <as-mac-1123@AS-MAC-1123.local> Co-authored-by: Alejandro Solares <219859296+ASolarers-Rodriguez@users.noreply.github.com> Co-authored-by: ViktorGo86 <114023094+ViktorGo86@users.noreply.github.com>
SUMMARY
Follow-up to #40832, which disabled the Discard button when a dashboard edit session has no unsaved changes. That fixed the misleading affordance (Discard had nothing to discard), but as @sadpandajoe noted on that PR, it left the edit-mode toolbar with no way back to view mode on a pristine session: Save and Discard are both disabled, and the "..." menu has no exit item, so the only escape is a page refresh or navigating away.
This keeps the button always enabled, and swaps the label based on state: Discard when there are unsaved changes, Exit edit mode when there are none. The click handler is identical in both cases (drops the
?editparam and returns to view mode), so the label always describes what actually happens.If @yousoph / @kasiazjc would rather see a different treatment (a separate exit control, different wording, etc.), happy to iterate. This is meant as the smallest change that removes the dead end.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: on a pristine edit session, Discard is disabled and there is no toolbar path back to view mode.
After: the same button reads "Exit edit mode" and is clickable; with unsaved changes it reads "Discard" exactly as before.
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
🤖 Generated with Claude Code