Only reload updated part of Layout Builder element

Added a test to cover adding, updating, and removing blocks.

pyrello → closed merge request !2951

Can a CR be created for the new trait?

Think the "Needs tests" can be removed too. Looks like you added those unless there are additional you're eyeing?

I added a draft change record. I've never written one before, so would very much welcome feedback.

I wasn't planning on any additional tests.

Updated issue summary.

Verified the issue using the browser inspector. I added 2 custom blocks
Edited 1 of the blocks and the entire form dom refreshed.

Applied the MR 4380

Verified that layout builder still functions as before. Tested by enabling for Article content type
Adding a custom block
Adding an Authored by field block
Moved some blocks around
Removed the custom block.
Added back.

Using the browser inspector and reusing the 2 custom blocks from before
Edited 1 of the blocks and see that just that div refreshed vs the entire form.

Fantastic!

Woah, epic work @pyrello!! 👏 Tagging for subsystem maintainer review in case one of the Layout Builder maintainers would like to review this.

I have a question here. By default LB works against the shared temp store until save is pressed.

In theory that means two people can edit the same layout at the same time.

Currently if someone makes one change, the 'reload everything' nature means that they would see each others changes.

With this change, they would not without a full page reload.

Do we think this is a use-case we need to think about?

Left a minor review on the MR. The tests are very clever, nice one.

In theory that means two people can edit the same layout at the same time.

It had never occurred to me that this was a feature of the reload everything approach.

Do we think this is a use-case we need to think about?

I have no data to back this up, but my guess is that most people have no idea that this is how LB works. I had never realized it and I have worked with LB extensively. Insofar as it is a feature, it is an imperfectly designed one because you are getting the "updates" from the other editor in bursts rather than in realtime. I'm not sure what the right answer is, but it seems like this patch would represent a choice to optimize for perceived performance/responsiveness at the expense of collaborative editing.

To provide a little more context, one of the motivations behind this change is to provide a path forward for potentially having in-place editing for Layout Builder. By providing the ability to update just a single block vs the entire LB object, it is theoretically easier to create such a system.

Should there be a feature that only 1 user can edit layout builder at a time?

Should there be a feature that only 1 user can edit layout builder at a time?

@smustgrave Can you elaborate on what you mean by that?

If the concern is around 2 people editing the same page. Could we implement some kind of checkout feature where only 1 user can edit a page layout at a time.

Moving to NW for the single trait mentioned by @larowlan.

Still not sure about the multiple users editing at once. Not sure if layout builder should support that or if it could be a contrib.

If the concern is around 2 people editing the same page. Could we implement some kind of checkout feature where only 1 user can edit a page layout at a time.

We have this for entities (give or take bugs #2892132: Entity validation does not always prevent concurrent editing of pending revisions → ) so it would not be unreasonable to implement it here too.

Here are a couple scenarios I have come up with to try to understand this a bit better. For each, there are two people, person A and person B, who are working in LB for a node at the same time.

Person A edits block X and person B edits block Y. Block X is saved slightly before block Y.

With current functionality:
After person A saves block X, person B will not see the changes to block X reflected in LB until after they have saved block Y. Person A will see the updates to block Y after they save the layout or make another change to a block.

With proposed functionality:
After person A saves block X, person B will not see the changes to block X reflected in LB until after they save the layout or try to edit block X.

Person A is editing block X and person B also starts editing block X before person A has saved their changes.

With current functionality:
Because person B already has the block form up, they do not see the changes to block X that person A saved. When they save the block, their changes become the new version of the block and their is no message to indicate that they have overridden person A's changes that happened since the last time they saw an up-to-date version of the node. Person A will either see person B's changes when they save the node or when they save changes to another block, or possibly not at all if they save the node prior to person B saving their changes to the block.

With proposed functionality:
The experience is the same here except for the case where person A edits another block. At that point they will not see person B's changes to block X because the whole layout doesn't get refreshed.

It seems like the simultaneous editing of layout feature has some issues that potentially cause unintended overwriting of content data. There is no warning or error to a user letting them know that the block they are working on or have just saved has changed data that they have never seen. It seems like regardless of whether the proposed functionality for this issue is adopted or not there should be some changes to warn or prevent a user from overwriting data they didn't mean it overwrite. Ideally, we should clarify whether two people are allowed to make layout changes simultaneously or not and make changes that support that decision.

We have this for entities (give or take bugs #2892132: Entity validation does not always prevent concurrent editing of pending revisions) so it would not be unreasonable to implement it here too.

Based on this, it seems like it would make sense to move towards functionality to not allow simultaneous editing of the same layout to be more internally consistent with how editing of entities is supposed to work.

It seems like the proposed functionality slightly exacerbates the disconnect between what two users working on the same node will know of each others changes. Is the degradation enough to warrant pausing this update until after we clarify the intended workflow for multiple users and implement changes to support/enforce that workflow? Or can we accept the imperfections to accept this change and then work on more intentional changes to the workflow afterwards?

Currently if someone makes one change, the 'reload everything' nature means that they would see each others changes.

With this change, they would not without a full page reload.

Do we think this is a use-case we need to think about?

I do think the behavior change is fine. I've talked to > 50 users about building pages with Layout Builder and Paragraph in the last 5 months and using the current tools for concurrent editing within Drupal hasn't come up in any of the workflows I've heard from them. Editing concurrently is a valid need but at the moment most users are collaborating outside of Drupal because we are not providing sufficient tooling for concurrent editing.

We have a pre-existing issue for introducing the locking: 🐛 Concurrent editing of layouts is very confusing Needs work to prevent concurrent editing (until we can support it properly).

So from that perspective, I think we should be good to move forward with the current proposal.

Setting back to "needs review" because I have a question for @larowlan about the proposed trait that this was set to needs work for.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.