fix(DynamicEditableTitle): preserve in-flight edits when title prop changes by msyavuz · Pull Request #39861 · apache/superset

msyavuz · 2026-05-04T16:42:38Z

SUMMARY

Follow-up to #38563. The effect at DynamicEditableTitle/index.tsx:88 calls setCurrentTitle(title) whenever the title prop changes — including mid-edit, so a parent re-render with a new title (autosave callback, optimistic update, sibling state push) clobbers the user's unsaved typing. The previous fix only addressed the handleChange !isEditing early-return, not this effect.

Gate the sync effect on !isEditing. handleBlur still calls setCurrentTitle(formattedTitle) before flipping isEditing false, so the consistency invariant is preserved on commit.

Also fix the existing regression test that didn't actually exercise this bug. It rerendered Harness with the same initialTitle="Foo", but Harness owns its own state and only reads initialTitle once — so the title prop the component received never actually changed. Rerender DynamicEditableTitle directly with a different title prop instead.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — preserves user input that was previously dropped.

TESTING INSTRUCTIONS

npx jest packages/superset-ui-core/src/components/DynamicEditableTitle/DynamicEditableTitle.regression.test.tsx --watchman=false — all 3 tests pass; the third one fails on prior HEAD if you revert only index.tsx.

ADDITIONAL INFORMATION

Has associated issue: No
Required feature flags: None
Changes UI: Yes — preserves in-flight typing in editable titles (dashboard/chart/tab/folder names) when parent re-renders mid-edit
Includes DB Migration: No
Introduces new feature or API: No
Removes existing feature or API: No

…hanges The effect at index.tsx:88 called setCurrentTitle(title) whenever the title prop changed — including mid-edit, which clobbered unsaved typing. Gate it on !isEditing so a parent re-render with a new title doesn't overwrite what the user has typed. handleBlur already syncs currentTitle on commit, so the consistency invariant is preserved. Also fix the existing 'prop changes mid-edit do not clobber' regression test, which rerendered Harness with the same initialTitle="Foo" — the prop the component received never actually changed, so the test passed even on broken code. Rerender DynamicEditableTitle directly with a different title prop so the sync effect actually runs.

bito-code-review · 2026-05-04T16:42:48Z

Code Review Agent Run #19886d

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 4bb6c8e..4bb6c8e
- superset-frontend/packages/superset-ui-core/src/components/DynamicEditableTitle/DynamicEditableTitle.regression.test.tsx
- superset-frontend/packages/superset-ui-core/src/components/DynamicEditableTitle/index.tsx
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 Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

Including isEditing in the dep array re-ran the sync effect when isEditing flipped from true to false on blur — which clobbered the just-committed value if the parent's title prop hadn't propagated yet (e.g. parents that don't update local state in the onSave callback, which is also the pattern in Header.test.tsx). Read isEditing via closure instead so the effect only fires when the title prop actually changes. handleBlur still syncs currentTitle on commit, so the consistency invariant is preserved.

rusackas · 2026-05-04T17:29:23Z

Thanks for cleaning this up — the gating is correct and the test now
actually exercises the bug it claims to (which it didn't before). One thing
worth confirming before merge:

The fix swaps one edge case for another. With if (!isEditing), this
scenario regresses:

title="Foo", user clicks the input (isEditing=true) without typing
Parent re-renders with title="Bar" (autosave from elsewhere, sibling
state push, etc.)
Effect skips, currentTitle stays "Foo"
User blurs without ever typing
handleBlur sees currentTitle="Foo" ≠ title="Bar" → calls onSave("Foo"),
silently reverting the parent's update

Before this PR, the user would have seen "Bar" appear and the blur would
have been a no-op. After this PR, opening edit mode is implicitly treated
as intent-to-overwrite, even when the user types nothing.

Probably still a net win over the original clobber bug, but if any flow
autosaves titles in the background (dashboards/charts/tabs/folder names)
this is worth a follow-up. Cheap fix: track a dirtyRef set inside
handleChange, and gate the onSave in handleBlur on it. Then passive focus
doesn't count as a write.

Smaller suggestion: extend the new test to also assert the commit
semantics, so a future change to handleBlur can't silently break "user
input wins":

rerender(<DynamicEditableTitle {...props} title="Bar" />);
expect(input.value).toBe('FooX');
fireEvent.blur(input);
expect(onSave).toHaveBeenCalledWith('FooX');

Otherwise LGTM.

codecov · 2026-05-04T17:42:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.37%. Comparing base (41a22d7) to head (65aea59).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #39861   +/-   ##
=======================================
  Coverage   64.36%   64.37%           
=======================================
  Files        2569     2569           
  Lines      134745   134746    +1     
  Branches    31278    31279    +1     
=======================================
+ Hits        86732    86738    +6     
+ Misses      46515    46510    -5     
  Partials     1498     1498

Flag	Coverage Δ
javascript	`66.62% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bito-code-review · 2026-05-04T18:31:16Z

Code Review Agent Run #d0eedc

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 4bb6c8e..65aea59
- superset-frontend/packages/superset-ui-core/src/components/DynamicEditableTitle/index.tsx
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 Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

rusackas

The phantom-revert in handleBlur - the onSave call is unconditional on title !== formattedTitle regardless of whether the user actually typed. Quick fix is a dirtyRef set in handleChange, gated in handleBlur before calling onSave (and reset).

Pairing it with the expect(onSave).toHaveBeenCalledWith('FooX') post-blur assertion in the regression test would lock the semantics in. Happy to approve once those are in.

pull-request-size Bot added the size/S label May 4, 2026

dosubot Bot added the packages label May 4, 2026

msyavuz requested a review from rusackas May 4, 2026 16:54

rusackas requested changes May 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(DynamicEditableTitle): preserve in-flight edits when title prop changes#39861

fix(DynamicEditableTitle): preserve in-flight edits when title prop changes#39861
msyavuz wants to merge 2 commits intomasterfrom
react18-title-clobber

msyavuz commented May 4, 2026

Uh oh!

bito-code-review Bot commented May 4, 2026 •

edited

Loading

Code Review Agent Run #19886d

Uh oh!

rusackas commented May 4, 2026

Uh oh!

codecov Bot commented May 4, 2026

Uh oh!

bito-code-review Bot commented May 4, 2026 •

edited

Loading

Code Review Agent Run #d0eedc

Uh oh!

rusackas left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

msyavuz commented May 4, 2026

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #19886d

Uh oh!

rusackas commented May 4, 2026

Uh oh!

codecov Bot commented May 4, 2026

Codecov Report

Uh oh!

bito-code-review Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #d0eedc

Uh oh!

rusackas left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bito-code-review Bot commented May 4, 2026 •

edited

Loading

bito-code-review Bot commented May 4, 2026 •

edited

Loading