Skip to content

Start the save block dialog as a new block and await the save - #505

Open
pierre-b wants to merge 11 commits into
devfrom
fix/save-block-modal
Open

pierre-b wants to merge 11 commits into
devfrom
fix/save-block-modal

Conversation

@pierre-b

@pierre-b pierre-b commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

What

Fixes the "Save Block" dialog of the email editor (floppy icon of the block toolbar).

  • It could open on overwriting another saved block. The dialog guessed whether the selected block was already saved by comparing its attributes without the long-hand paddings, so two white sections were enough for it to open in "Update existing block" mode with an unrelated saved block preselected. One click on Update replaced that library entry for the whole workspace. It now always opens on "Save as new block".
  • "Update existing block" listed every saved block, so a saved section could be replaced by a button. It now lists only saved blocks of the selected block's type, newest first, without sorting the prop in place, and is disabled when there are none.
  • The confirmation did not reflect the save. The toolbar showed its own success message without awaiting the save, then the drawer showed a second one, or an error after the first if the API call failed, with the dialog already closed. The toolbar now awaits the save: it closes on success, keeps the dialog open with what was typed on failure, and cannot be dismissed while a save is pending. The drawer's handler rejects when the API call fails; a failed workspace refresh after a successful save shows a warning instead of an error.

SaveBlockHandler (returns Promise<void>) replaces the inline signature at the six places that pass the handler down. The unused delete branch of the drawer's handler goes, since SaveOperation is 'create' | 'update'.

Plan: plans/fix-save-block-modal-plan.md.

Tests

  • BlockToolbar.test.tsx (new, 9 tests): new mode with a look-alike saved section, same-type update list, disabled update, prop not reordered, close after resolve with no toolbar toast, stays open on failure, cannot be dismissed while pending (Cancel, Escape, mask), update call, Enter during a pending save.
  • CreateTemplateDrawer.test.tsx (+4): create with one success message, API failure rejects, missing block rejects without calling the API, refresh failure resolves with a warning.
  • e2e/features/saved-blocks.spec.ts (new, Playwright, API mocked): look-alike section saved as new, update lists sections only, failed save keeps the dialog open. Run locally (CI does not run Playwright).
  • Full console suite, tsc -b, eslint, npm run build; make test-unit.

Changelog

Entry added to the unreleased 43.2 section.

Known, not handled

  • The docs say editing a saved block changes every template that embeds it (features/admins-api-keys.mdx, empty.json); blocks are inserted as copies. Separate docs PR.
  • de.po shows literal backslashes in Vorlagenblock \"{nameOrId}\" erfolgreich gespeichert (predates this PR).
  • web_analytics_sdk/package-lock.json root version still says 42.3.0 (predates this PR).
  • A stalled workspace refresh keeps the dialog in its saving state (no request timeout in the API client).
  • invalidateQueries({ queryKey: ['workspace', id] }) matches no query in the console.
  • Default name section Block, raw mj-section type, no preview, no rename or delete in the editor: saved-blocks library work that follows.

The dialog guessed whether the selected block was already saved by comparing
its attributes minus long-hand paddings, so two white sections were enough
for it to open on updating another saved block. It now always starts as a
new block, and "Update existing block" lists only saved blocks of the same
type, newest first, without sorting the prop in place.

The toolbar awaits the save: it closes once the block is saved, keeps the
dialog open with what was typed when saving fails, cannot be dismissed while
a save is pending, and no longer shows its own success message next to the
drawer's. The drawer's handler rejects when the API call fails, and a failed
workspace refresh after a successful save shows a warning instead of an
error. The unused delete branch goes with the typed handler.

Version 43.1, web analytics SDK synced.
Each Enter in the pending-save test is pressed and released, since the input
ignores a second keydown until keyup. The dismissal test also presses Escape
and clicks the mask. The e2e failure case saves a second time instead of
reading a dialog that may only be closing. The dialog uses mask.closable,
and the update select shows its placeholder.
# Conflicts:
#	CHANGELOG.md
#	console/src/i18n/locales/ca.po
#	console/src/i18n/locales/de.po
#	console/src/i18n/locales/en.po
#	console/src/i18n/locales/es.po
#	console/src/i18n/locales/fr.po
#	console/src/i18n/locales/it.po
#	console/src/i18n/locales/ja.po
#	console/src/i18n/locales/pt-BR.po
@pierre-b pierre-b self-assigned this Nov 10, 2026
@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.82%. Comparing base (ffa835f) to head (d847228).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #505   +/-   ##
=======================================
  Coverage   88.82%   88.82%           
=======================================
  Files         368      368           
  Lines       71862    71862           
=======================================
+ Hits        63828    63831    +3     
+ Misses       5395     5392    -3     
  Partials     2639     2639           
Flag Coverage Δ
unit 88.82% <ø> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

…hovering

Before the fix a second Enter saved the block twice; the reset name did not stop it. The dismissal test now
opens the dialog with a plain click, so the save button's tooltip cannot take the dialog off the Escape stack
and leave the Escape check without effect. The plan follows the code (mask.closable, the Escape and mask
checks, no Go or SDK change to verify).
@pierre-b
pierre-b marked this pull request as ready for review November 10, 2026 20:56
# Conflicts:
#	console/src/i18n/locales/ca.po
#	console/src/i18n/locales/de.po
#	console/src/i18n/locales/en.po
#	console/src/i18n/locales/es.po
#	console/src/i18n/locales/fr.po
#	console/src/i18n/locales/it.po
#	console/src/i18n/locales/ja.po
#	console/src/i18n/locales/pt-BR.po
The changes merged since 43.0 and the open ones ship together in 44.0:
the 43.1 and 43.2 changelog sections become one 44.0 section, and the
code and web analytics SDK versions move to 44.0.

This branch has not been deployed

No deployments
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.

1 participant