Skip to content

fix(superdoc): expose header/footer edits in update callbacks#2368

Open
luccas-harbour wants to merge 5 commits intomainfrom
luccas/sd-2179-bug-headerfooter-edits-do-not-emit-update-events-until-blur
Open

fix(superdoc): expose header/footer edits in update callbacks#2368
luccas-harbour wants to merge 5 commits intomainfrom
luccas/sd-2179-bug-headerfooter-edits-do-not-emit-update-events-until-blur

Conversation

@luccas-harbour
Copy link
Contributor

Summary

This PR makes header and footer edits observable through the same public callback surface as body edits.

Before this change, typing in header/footer child editors updated internal state, but those edits were not propagated through the public onEditorUpdate and onTransaction hooks with the same immediacy as body edits. This caused integrations that rely on those callbacks for dirty-state tracking, autosave, and other live reactions to miss header/footer changes until blur.

What Changed

  • Forward live header/footer child-editor update and transaction events through the header/footer session flow.
  • Re-emit those events from PresentationEditor as header/footer-specific presentation events.
  • Bridge those presentation events into the existing SuperDoc public callback surface.
  • Normalize callback payloads so callers can tell where the change originated via:
    • sourceEditor
    • surface (body, header, or footer)
    • headerId
    • sectionType
  • Update the published SuperDoc and React wrapper event types to match the new payload shape.

Testing

  • Added a PresentationEditor test that verifies live header/footer child-editor updates and transactions are re-emitted immediately.
  • Added a SuperDoc.vue test that verifies header/footer presentation events are forwarded into editor-update and onTransaction.

@linear
Copy link

linear bot commented Mar 11, 2026

Copy link
Contributor

@caio-pizzol caio-pizzol left a comment

Choose a reason for hiding this comment

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

@luccas-harbour clean approach, event listeners are set up and cleaned up in all the right places. no correctness blockers.

left three small inline comments — one consistency fix and two simplification ideas.

on tests: the happy path is covered well. three gaps worth considering: (1) no test that events stop firing after leaving header/footer mode, (2) no test for the body-editor path through the new payload builders, (3) footer surface isn't tested end-to-end (only header).

* Clean up all resources.
*/
destroy(): void {
this.#teardownActiveEditorEventBridge();
Copy link
Contributor

Choose a reason for hiding this comment

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

the resetSession callback in initialize() sets #activeEditor = null without calling #teardownActiveEditorEventBridge() first. every other place that clears #activeEditor does the teardown. not a bug since the editor is already destroyed, but the leftover cleanup ref could log a warning next time around. worth adding for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, done

this.#activeEditorEventCleanup = () => {
try {
editor.off?.('update', emitSurfaceUpdate);
} catch (error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the individual try/catch blocks around each editor.off() are heavier than anywhere else in the codebase — other spots just call editor.off() directly. the outer try/catch in #teardownActiveEditorEventBridge already covers this. ok to simplify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, I simplified this


const onEditorUpdate = ({ editor }) => {
proxy.$superdoc.emit('editor-update', { editor });
const buildEditorUpdatePayload = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

these two builder functions (buildEditorUpdatePayload and buildEditorTransactionPayload) do the same thing for the first 5 fields — only transaction and duration differ. could pull the shared part into one helper to keep them in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, I refactored this to use a common helper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants