Overview
Follow up to #3491459: Implement the "Review N changes" button
Proposed resolution
User interface changes
Issue fork experience_builder-3500390
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3500390-pending-changes-individual-regions
changes, plain diff MR !572
Comments
Comment #2
effulgentsia commentedBesides changing this endpoint, is there any other complication we can foresee with breaking the 1:1 relationship between what the UI shows in the pending changes list and what corresponds to an entity? Hopefully, it's no big deal, but just wanting to ask before we go down that road.
Comment #3
larowlan@effulgentsia
Rough implementation plan:
saveGlobalRegion- this just appends the region name to the existing key and labelSo whilst the title says 'pending changes api' the actual changes are elsewhere.
Comment #4
larowlanComment #7
longwaveComment #8
wim leers#3: #3500087: Make AutoSaveManager::getAutoSaveKey static already landed Jan 17 :)
Comment #9
larowlanComment #10
larowlanCrosspost
Comment #11
larowlanComment #12
wim leersPer https://git.drupalcode.org/project/experience_builder/-/merge_requests/5..., I'd like @longwave's thoughts here in particular because it's thanks to him we have
docs/adr/0005-Keep-the-front-end-simple.md, and @larowlan's remark reveals that here we're deviating from that.Comment #13
longwaveComment #14
effulgentsia commentedI like the approach here and there's no need to expand this issue's scope. Regarding my comment #2, I opened #3501542: Allow a wse_config entity to represent a config partial rather than a complete config entity.
Comment #15
longwaveJust thinking out loud a bit, but are we over complicating by trying to split the config entity both here and eventually in Workspaces? We created the config entity - should page_template be a page_region config entity instead? This would simplify all the cases we have so far - perhaps there are downsides I haven't thought of.
Comment #16
larowlanI think it's worth exploring that idea. I'm not sure if this is the issue for that or we should postpone this on a spike
Comment #17
effulgentsia commentedI opened #3501600: Split 1 PageTemplate config entity into N PageRegion config entities.
Comment #18
wim leersPer #3501600-7: Split 1 PageTemplate config entity into N PageRegion config entities, I now think it's quite likely we should not do this, and do that instead. Postponing until #3501600 lands and we're 100% confident we won't need (something like) this.
@longwave: well predicted! I see at least one nice bonus benefit — see bottom of that comment 😄
Comment #19
effulgentsia commentedComment #20
wim leers99% certain that we're going with #3501600 per #3501600-13: Split 1 PageTemplate config entity into N PageRegion config entities, just needs @lauriii confirmation for me to tackle this next week 👍
Comment #21
wim leers#3501600: Split 1 PageTemplate config entity into N PageRegion config entities landed.
Comment #22
wim leersThat is indeed the remaining challenge after #3501600: Split 1 PageTemplate config entity into N PageRegion config entities:

But … this is a problem already for the content entity's XB field already. For that, we have #3502902: Only auto-save content entities/PageRegion config entities when there are actual changes: simply previewing incorrectly causes them to appear in "Review x changes". Both the edited content entity and the global regions (1
PageTemplatewith N component trees, and as of minutes ago, NPageRegionconfig entities with 1 component tree each) get their auto-save states created through\Drupal\experience_builder\Controller\ApiLayoutController::buildPreviewRenderable(), so it's all already in a single place already anyway.Hence my proposal to tackle that all in #3501600: Split 1 PageTemplate config entity into N PageRegion config entities.
Comment #24
nagwani commented