Skip to content

Make the Undo button inert when there is nothing to undo - #6082

Merged
raineorshine merged 6 commits into
mainfrom
copilot/fix-greyed-out-undo-button
Oct 23, 2026
Merged

raineorshine merged 6 commits into
mainfrom
copilot/fix-greyed-out-undo-button

Conversation

Copilot AI commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Tapping the greyed-out Undo button with an empty undo stack still played the undo animation. It should do nothing on any platform while the stack is empty.

Cause

Since #4013, undo declares isActive: isUndoEnabled in place of canExecute. Without canExecute, the command always counts as executable, so Cmd+Z always calls preventDefault and blocks native undo (#3879). Two side effects on the toolbar:

  • Undo always counts as executable, so tapping it while greyed out runs a no-op undo and animates the icon.
  • ToolbarButton animates a command that has isActive only when it goes from inactive to active. As a result, a real undo never animated, and the animation only ever played on the wrong tap.

Change

  • src/commands/undo.ts:

    • Restores canExecute: isUndoEnabled.
    • Adds the existing preventDefault: true flag, which keyDown honours even when canExecute is false, so Cmd+Z still always swallows native undo.
    • Drops isActive.
    canExecute: state => isUndoEnabled(state),
    // Always call e.preventDefault() to block native undo, even when there is nothing to undo.
    preventDefault: true,
  • Resulting behaviour:

    • Toolbar and Command Center: the Undo button can't be tapped while the stack is empty.
    • The command palette lists Undo as disabled while the stack is empty.
    • A successful undo now animates the icon, matching Redo.
    • Undo no longer gets the Command Center "active" glow, matching Redo.
    • Unchanged: native history gestures (handleNativeHistory checks isUndoEnabled directly) and long-press opening the UndoSlider.

Test

  • src/e2e/puppeteer/__tests__/undo.ts (mobile):
    • Taps Undo on an empty stack and expects zero animation mounts.
    • Then makes an edit and expects exactly one mount when Undo is tapped. This positive control shows the first tap actually reached the button.
    • Counts mounts with a MutationObserver, because under Puppeteer LottieAnimation jumps to its last frame and unmounts as soon as it's ready. Reading the DOM after the tap would miss it.

#5994)

Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor
Preview Deployment · Oct 23, 2026, 12:14 AM UTC · 5a5ac4f

Preview deployment

Restore canExecute on undo so the greyed-out toolbar and Command Center buttons are inert, and set preventDefault so Cmd+Z still always blocks native undo (#3879).

Co-Authored-By: GitHub Copilot Coding Agent unknown (unknown) <223556219+Copilot@users.noreply.github.com>

Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix greyed-out Undo button tap behavior Make the Undo button inert when there is nothing to undo Oct 21, 2026
Copilot AI requested a review from BayuAri October 21, 2026 03:36

@BayuAri BayuAri left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Confirmed fixed.
Undo button is disabled on first open and when there is no Undo Stack to execute accordingly

@BayuAri
BayuAri requested a review from trevinhofmann October 21, 2026 04:27
@BayuAri
BayuAri marked this pull request as ready for review October 22, 2026 02:24
Comment thread src/commands/undo.ts
if (!isUndoEnabled(getState())) return
dispatch(undo())
},
canExecute: state => isUndoEnabled(state),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This change results in the Undo toolbar button being disabled for ~500ms on the first edit due to the text edit throttling.

Can you please resolve this? One option to consider is updating Editable.tsx to commit the first edit immediately by adding isUndoEnabled as one of the conditions here:

wrappedValue !== incomingValue ||
emojiSpaceAdded ||
transient ||
contextLengthChange ||
urlChange ||
formattingChange ||
isEmpty ||
isDivider(newValue)

Case to reproduce below (please also add as a test case, if feasible... not sure if we have a nice way to simulate reloading the app / starting with a thought but an empty history):

  1. Create a thought Hello
  2. Reload the application
  3. Place the cursor at the end of Hello and type W (becoming HelloW)

Result: The "Undo" button is disabled for ~500ms until becoming active and working

Expected: The "Undo" button should become immediately active, as it does in main.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 3f1deef. I used your suggestion: Editable now commits an edit immediately when !isUndoEnabled(state), so the first keystroke after a reload enables Undo right away.

  • Undo grouping: unchanged. Typing World after a reload and tapping Undo once still returns to Hello, because the undo history merges the immediate edit with the throttled edits that follow it.
  • Test: added to src/e2e/puppeteer/__tests__/undo.ts (mobile, persistent storage). It reloads, types W at the end of Hello, taps Undo right away, and expects Hello. Before the fix it failed with expected 'HelloW' to be 'Hello'.
  • Side effect on desktop: the first keystroke after a reload now starts the distraction-free toolbar fade about 500ms sooner, the same as edits that already commit immediately (URLs, formatting).
  • Docs: docs/commands.md "Flushing pending edits" now describes this exception to the throttle.

@trevinhofmann

Copy link
Copy Markdown
Collaborator

@copilot - Can you please address this review comment and resolve the defect?

#6082 (comment)

Co-Authored-By: GitHub Copilot Coding Agent unknown (unknown) <223556219+Copilot@users.noreply.github.com>

Co-authored-by: trevinhofmann <5055041+trevinhofmann@users.noreply.github.com>
Undo's canExecute reads the undo history, so the edit throttle left the
Undo button disabled for ~500ms after the first edit following a reload.

Co-Authored-By: GitHub Copilot Coding Agent unknown (unknown) <223556219+Copilot@users.noreply.github.com>

Co-authored-by: trevinhofmann <5055041+trevinhofmann@users.noreply.github.com>

@trevinhofmann trevinhofmann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thank you!

The Lottie animation is a side effect of the enabled state, and the tests were big, slow, and coupled to animation details.

Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
Comment thread src/e2e/puppeteer/__tests__/undo.ts Outdated
})

// https://github.com/cybersemics/em/issues/5994
it('does not animate the greyed-out Undo button when there is nothing to undo', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The Lottie animation is a side effect of the undo button being enabled when it shouldn’t. While it makes sense as a proxy for the actual issue, it is not a good candidate for automated testing. It’s big and slow, and too tightly coupled to animation implementation details. Unfortunately the agent didn’t pick up on this distinction.

I would completey remove both of these tests.

5a5ac4f

@raineorshine
raineorshine merged commit d0967c3 into main Oct 23, 2026
12 checks passed
@raineorshine
raineorshine deleted the copilot/fix-greyed-out-undo-button branch October 23, 2026 00:31

This branch was successfully deployed

1 active deployment
Preview — 5a5ac4f9 Deployed Oct 23, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Toolbar: Greyed-out Undo button can be tapped when there is nothing to undo

4 participants