#3922 - Macro: After undoing and redoing all steps, Undo and Redo buttons do not turn gray, as they do in Micro mode. - #11903
Open
mariam-khutuashvili wants to merge 6 commits into
Conversation
…tons do not turn gray, as they do in Micro mode The macromolecules top menu rendered Undo and Redo as enabled whenever the editor was usable, regardless of whether there was anything to undo or redo. Micro mode greys them out at either end of the history; macro mode never did. Read the history through useSyncExternalStore: subscribe to the editor's `modelChange` event and derive `canUndo`/`canRedo` from `EditorHistory`'s pointer and stack. The read is synchronous during render, so the buttons are right from the first paint - an effect would leave both briefly disabled on a canvas that does have a history - and the component only re-renders when one of the two booleans actually changes.
…of the history Two tests only passed because the buttons were wrongly enabled. With the buttons now disabled at either end of the history, a click there times out. `undo-redo.spec.ts` "Undo redo for monomers and bonds addition" clicked Redo twice up front and Undo five times later on to "stop on the first operation". Assert that Redo is disabled while nothing has been undone, and undo while the button is enabled instead of guessing a count. Also drop the note on "Check that after undoing and redoing all steps ..." saying the test could not work until #3922 was fixed - that is this change. `sequence-mode-copy-paste.spec.ts` "Select letters with Shift & ArrowLeft then paste ket from clipboard and undo" undid twice after a copy for no reason; remove those steps and the screenshot that depended on them.
…edo buttons Undo and Redo now render greyed out at either end of the history, so every page screenshot of the macromolecules editor taken with nothing to undo or redo changes by those two icons. Baselines are the first-attempt actuals of the CI run on 184890a (runs 34836062970 and 34836065787), taken from the blob reports and mapped to their files through the sha1 of each expected copy. Every file was audited before being written: the change is confined to a 14-47 px wide, 5-6 px tall box at the Undo/Redo position of the toolbar, and nothing else moved. Not included, on purpose: - macro-micro-switcher "Mol-/Ket-structure opened in Macro mode is visible in Micro mode": a micro-canvas offset that is byte-identical with and without this fix, so it is not caused by it. - snake-bond-tool "Check snake mode arrange for peptides chain": a canvas diff from a spec known to be unstable. - Tests that passed on retry. Tests that take several screenshots stop at the first mismatch, so their later screenshots are refreshed in the next CI round.
…led Undo/Redo buttons Second round of the same harvest, from the CI run on d9d5551 (runs 35061170508 and 35061172909). A test stops at its first screenshot mismatch, so the tests that take several screenshots only exposed their next one once the first had been refreshed. Same mapping through the sha1 of the expected copy, same audit: each change is a 14-47 px wide, 5-6 px tall box at the Undo/Redo position of the toolbar.
…led Undo/Redo buttons Third screenshot of "Case 8: clicking the icon when the window is opened, or the X, closes that window", exposed only once its first two had been refreshed. Same source (CI run 35063589284 on 8939af0), same sha1 mapping, same audit: a 14 px wide, 5 px tall box at the Undo position of the toolbar.
…age screenshots that depend on the leftover history
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cc.mp4
In Macro mode the Undo and Redo buttons rendered as enabled whenever the editor was usable, regardless of whether there was anything to undo or redo. Micro mode greys them out at either end of the history; Macro mode never did.
TopMenuComponent now reads the history through useSyncExternalStore: it subscribes to the editor's modelChange event and derives canUndo/canRedo from EditorHistory's pointer and stack. The read is synchronous during render, so the buttons are correct from the first paint, and the component only re-renders when one of the two booleans actually changes.
Two autotests only passed because the buttons were wrongly enabled — they clicked Undo/Redo past the ends of the history, which now times out on a disabled button. undo-redo.spec.ts asserts Redo is disabled while nothing has been undone and undoes while the button is enabled rather than a fixed count; sequence-mode-copy-paste.spec.ts drops two pointless undo steps after a copy. The stale note on the "buttons turn gray" test saying it could not work until #3922 was fixed is removed.
Screenshot baselines (69 files) were taken from this branch's own CI runs — first-attempt actuals, mapped through the sha1 of each expected copy, and each audited to confirm the change is confined to a 14–47 × 5–6 px box at the Undo/Redo position of the toolbar. Tests that take several screenshots stop at the first mismatch, so the set was completed over three CI rounds.
Deliberately not rebaselined, since they are unrelated to this change:
Check list
#1234 – issue name