Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
raineorshine merged 6 commits into from
Oct 23, 2026
+8 −2
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
494eccf
Initial plan
Copilot Oct 21, 2026
1b6305a
Add skipped test: greyed-out Undo button animates with nothing to und…
Copilot Oct 21, 2026
d81ceb8
Disable the Undo command when there is nothing to undo (#5994)
Copilot Oct 21, 2026
478dd56
Add skipped test: Undo works on the first edit after a reload
Copilot Oct 22, 2026
3f1deef
Commit the first edit immediately when there is nothing to undo
Copilot Oct 22, 2026
5a5ac4f
Remove Undo button e2e tests
raineorshine Oct 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/commands.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ Both take the browser selection away from the thought as they open — the deskt

Typing into a thought does not reach Redux on the keystroke. [`Editable`](../src/components/Editable.tsx) dispatches `editThought` through a lodash throttle of `EDIT_THROTTLE` (500 ms) with `leading: false`, so the value the user just typed sits on the trailing edge until the window closes. A command that mutates thought text and runs inside that window would read the pre-edit value, and the trailing edit would then commit over the command's own result — the typed character wins and the formatting is lost ([issue #4774](https://github.com/cybersemics/em/issues/4774)).

The exception is an edit typed while there is nothing to undo, such as the first keystroke after a reload, which `Editable` commits immediately. Undo's `canExecute` reads the undo history, so the throttle would otherwise leave the Undo button disabled for the first 500 ms of typing, and a tap on it in that window would be dropped. The edits that follow are throttled as usual, and the undo history merges them with the immediate one into a single step.

`Editable` subscribes to `commandEmitter` and flushes the throttle on `command`, so any activation surface can force the pending edit to commit first by triggering it. Every surface does, at the point where it knows a command is about to run:

- **Keyboard and gesture** — `keyDown` and `handleGestureEnd` trigger it before they dispatch.
Expand Down
5 changes: 3 additions & 2 deletions src/commands/undo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ const undoCommand = {
if (!isUndoEnabled(getState())) return
dispatch(undo())
},
canExecute: state => isUndoEnabled(state),

@trevinhofmann trevinhofmann Oct 22, 2026

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:

em/src/components/Editable.tsx

Lines 848 to 855 in a80340a

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.

Copilot AI Oct 22, 2026

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.

// Native browser undo creates problems when document.execCommand has been used for formatting. (#3879)
// canExecute should always be true so that it always calls e.preventDefault() and blocks native undo.
isActive: state => isUndoEnabled(state),
// Always call e.preventDefault() to block native undo, even when there is nothing to undo.
preventDefault: true,
longPress: dispatch => {
dispatch(toggleDropdown({ dropDownType: 'undoSlider' }))
},
Expand Down
3 changes: 3 additions & 0 deletions src/components/Editable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import getThoughtById from '../selectors/getThoughtById'
import hasMulticursorSelector from '../selectors/hasMulticursor'
import isMultiEditing from '../selectors/isMultiEditing'
import isMulticursorPath from '../selectors/isMulticursorPath'
import isUndoEnabled from '../selectors/isUndoEnabled'
import rootedParentOf from '../selectors/rootedParentOf'
import simplifyPath from '../selectors/simplifyPath'
import thoughtToPath from '../selectors/thoughtToPath'
Expand Down Expand Up @@ -844,7 +845,9 @@ const Editable = ({

// run the thoughtChangeHandler immediately if superscript changes or it's a url (also when it changes true to false)
// run it immediately is there is a style wrapper that needs to be applied to the editable after a clearThought action (#3673)
// run it immediately if there is nothing to undo yet, so that the Undo button is enabled as soon as the first edit is typed rather than after the throttle (#5994)
if (
!isUndoEnabled(state) ||
wrappedValue !== incomingValue ||
emojiSpaceAdded ||
transient ||
Expand Down
Loading