Skip to content

Fix command menu text/number inputs to commit on blur and cancel cleanly on Escape#18283

Open
ehconitin wants to merge 2 commits intomainfrom
fix-input-on-blur
Open

Fix command menu text/number inputs to commit on blur and cancel cleanly on Escape#18283
ehconitin wants to merge 2 commits intomainfrom
fix-input-on-blur

Conversation

@ehconitin
Copy link
Contributor

@ehconitin ehconitin commented Feb 26, 2026

closes #18264

CleanShot.2026-02-27.at.00.14.33.mp4
CleanShot.2026-02-27.at.00.16.19.mp4

PR description -

This fixes flaky persistence in command menu text and number inputs.

  • moved commit logic to onBlur (single commit path)
  • Enter now blurs, so it uses the same commit path
  • Escape now cancels edit (restores draft + exits) without persisting
  • removed dependency on input click-outside commit timing

Outcome -

  • clicking anywhere outside the input now reliably persists edits
  • Escape consistently discards edits

Copy link
Contributor

@cubic-dev-ai cubic-dev-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.

No issues found across 2 files

@ehconitin ehconitin self-assigned this Feb 26, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR consolidates input commit logic in command menu text/number inputs by using onBlur as the single commit path, improving reliability and eliminating race conditions with click-outside handlers.

Key Changes:

  • Moved from useAtomStateValue to useStore for synchronous focus state checks in handleBlur
  • Enter key now triggers blur (which commits), ensuring single code path for persistence
  • Escape key removes from focus stack before blurring, preventing unwanted commits
  • Removed onClickOutside handler and its race-condition-prone timing dependency

Architecture:
The refactored logic uses the focus stack state to distinguish between different blur scenarios: when handleBlur fires, it checks if the input is still in the focus stack - if yes, it's a "real" blur (click-outside or Enter) and should commit; if no, it's from Escape and should not commit.

Issue Found:
One minor bug in CommandMenuItemNumberInput: the hasError state persists after Escape resets the value, causing an error indicator to show on a valid value until the user types again.

Confidence Score: 4/5

  • This PR is safe to merge after fixing the minor error state bug
  • The refactoring improves code reliability by eliminating race conditions and centralizing commit logic. The implementation is sound and consistent across both components. One minor logic bug (error state not cleared on Escape) should be fixed, but doesn't affect core functionality.
  • CommandMenuItemNumberInput.tsx needs the error state clearing fix in handleEscape

Important Files Changed

Filename Overview
packages/twenty-front/src/modules/command-menu/components/CommandMenuItemNumberInput.tsx Refactored input commit logic to use onBlur as single commit path, removed click-outside handler. Minor bug: hasError state not cleared on Escape.
packages/twenty-front/src/modules/command-menu/components/CommandMenuItemTextInput.tsx Applied same refactoring as NumberInput - simplified commit logic by centralizing to onBlur handler. Clean implementation with no issues found.

Last reviewed commit: 4d384b3

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@bosiraphael bosiraphael left a comment

Choose a reason for hiding this comment

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

LGTM

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard - prefix displayed is not the one saved

2 participants