Conversation
jasonvarga
left a comment
There was a problem hiding this comment.
I'm happy to update this myself, but I'm just gonna dump this AI review:
Warning: Missing localization for "Save All"
The new button text __('Save All') has no translation entries in any language file. The old strings it replaces ("Apply & Close All" and "Save & Close All") had translations in at least 5 languages (de, de_CH, fr, nl, ru). Non-English users will see untranslated English text.
Warning: softSave and commitAndSaveTopStack call saveRootForm() for nested fields, but changes haven't propagated to the root form
Both softSave() and commitAndSaveTopStack() pass shouldSaveRoot: true, which triggers saveRootForm() → emits root-form-save → the Builder/fieldset form saves.
For a top-level field this works correctly: the committed event synchronously updates the RegularField → blueprint data before saveRootForm() fires.
For a nested field (e.g. a field inside a replicator set), the committed event only updates the parent Settings' internal values state. The parent Settings hasn't committed back to the blueprint data yet. So saveRootForm() persists the blueprint in its pre-edit state — the nested field's changes won't actually be on disk until the parent stack is also saved.
This isn't data loss (changes stay in memory), but it fires an unnecessary HTTP request and could mislead users into thinking their changes are persisted when they're not. Consider either:
- Removing
shouldSaveRoot: truefromcommitAndSaveTopStack(it only needs to close the current stack), or - Only setting
shouldSaveRootwhen!this.isNestedField
Note: commitAndSaveTopStack is functionally identical to commitAndSave
Both methods call this.commit({ shouldSaveRoot: true }) with no other differences. Consider consolidating or having one delegate to the other.
Note: saveAllTooltipText is a trivial wrapper
The computed saveAllTooltipText just returns this.saveAllShortcutLabel with no added logic. Could be inlined directly in the template as v-tooltip="saveAllShortcutLabel".
- Change commitParentField inject default from () => {} to null so the
top-level Settings correctly falls through to saveRootForm()
- Add shouldEmitCommitted flag to commit() for future flexibility
- For new fields, Cmd+S delegates to the Save button behavior (create
and close)
- For existing nested fields, Cmd+S commits the parent chain and saves
the root form without closing any stacks
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Description of the Problem
Related to #12093, There's been ongoing discussion about how the button row should behave for blueprints—see the feedback thread for full context.
In short:
What this PR Does
Simplifies the button row to a maximum of two buttons, with consistent keyboard shortcuts throughout.
Single-level stacks show one button:
Multi-level stacks show two buttons:
Keyboard shortcuts (available everywhere):
cmd+s—soft-save, keeping the stack open (consistent with behaviour elsewhere in the control panel).cmd+shift+s—save all, closing every open stack. Works on single stacks too.A tooltip on the Save All button aids discoverability for users who haven't encountered the shortcut yet.
How to Reproduce