Fix writer update issue

[bastianallgeier]

Description

The writer input records transactions for actual state updates but also for selections. So far, we don't check before we emit an input event. This leads to a quite nasty situation where selections or even just a focus even also emit an input event at the same time. With the new changes setup, this means that every selection change or focus event will send an update to the backend. This PR fixes this by checking for the involved steps in the transaction object. If there are no steps, there's no need to emit something.

Changelog

Fixes

• [v5] Block fields preview: content.save event is triggered although content hasn’t changed #6959

Breaking changes

None

For review team

• Add changes & docs to release notes draft in Notion

[afbora]

Brilliant!

[bastianallgeier]

@afbora I've just made another change. We never really watch the preventUpdate meta data entry in the update method. But this is set on purpose in the value watcher to avoid sending additional input events when the value changed from the backend.

Before, hitting the discard button would always lead to another input event being triggered by the writer. After this change, the additional input events are gone.

[afbora]

@bastianallgeier Actually we watch it. Is this same?
https://github.com/getkirby/kirby/blob/v5/fix/6959-writer-update-issue/panel/src/components/Forms/Writer/Editor.js#L288

[bastianallgeier]

Oh, this is super weird. You are right. But it seems that it somehow still propagates to the update event. Then I have to track down why it is not being detected there.

[afbora]

OK. Also we can move the payload.transaction.steps.length check into Editor.js::dispatchTransaction() method to consistency.

[bastianallgeier]

The update event seems to be triggered from somewhere else and I can't find it right now. It isn't enough to make that check in the dispatchTransaction method. Update still gets called.

I've also made another change. We trigger an input event when the editor is being initialized. This seems very wrong to me. I think we did this earlier when the HTML from the backend would be different from what the writer would create as document. In that case the values between backend and changes in local storage would not be exactly the same and the changes button would appear. But I think we don't really need this anymore with the new setup. But we have to test it some more.

[bastianallgeier]

I just tested it with invalid HTML on the backend and it works flawlessly. I really think that the combination of cleaned up events and the new changes architecture will finally help us tame the Writer monster.

[afbora]

@bastianallgeier I think if condition is wrong, it should be like that: use and operator instead or. Just following changes works for me.

// Only emit an update if the doc has changed and
// an update has not been actively prevented
if (
  transaction.docChanged &&
  !transaction.getMeta("preventUpdate") &&
  transaction.steps.length > 0
) {
  this.emit("update", payload);
}

https://github.com/getkirby/kirby/blob/v5/fix/6959-writer-update-issue/panel/src/components/Forms/Writer/Editor.js#L288

[bastianallgeier]

This is embarrasing. You are totally right. The || broke the logic there 🙈 Works like a charm now.