Closed (fixed)
Project:
Experience Builder
Version:
0.x-dev
Component:
Data model
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
15 Dec 2024 at 23:28 UTC
Updated:
21 Jan 2025 at 10:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
larowlanComment #3
wim leers#3492669: Use PageTemplate status flag to enable per-theme page templates is in!
Comment #4
larowlanComment #7
longwaveThis has #3489899: Add support for global regions merged in and will make more sense once that has landed in 0.x.
At present autosave and reload of page templates seems to work, next steps is to actually render the autosaved page template, plus the saved model needs adjusting to extract only the parts relevant to the content entity and page template entity instead of saving the entire thing.
Comment #8
wim leers#3489899: Add support for global regions is in!
Comment #9
longwaveRebased following #3489899: Add support for global regions being merged.
Comment #10
longwaveI've tied this in with #3492669: Use PageTemplate status flag to enable per-theme page templates and ensured that:
Comment #11
wim leers#10++ — nice! 😄
Many questions on the MR, but this is definitely going in the right direction! 👍
Assigning to @longwave because he's actively working on it, and Lee is hopefully asleep 😊
Comment #12
larowlanComment #13
longwaveAssigning to Wim for review, I've reviewed this and think that we are in good shape with it, and can't see anything left to do except @todos that are already tagged.
I do think that we could try and remove the root UUID now as it's a bit meaningless, we do special steps to ensure it exists, but why does it have to exist at all? Opened #3495625: Remove ComponentTreeItemList::ROOT_UUID from hydration and client-to-server conversion to investigate that.
Comment #14
effulgentsia commentedThis looks fantastic, but an E2E test is now failing. Possibly due to a recent commit to 0.x?
Comment #16
effulgentsia commentedI might be wrong, but I think Wim is off work this whole week for the holidays, so I reviewed this in his stead, and it looks great to me, so I merged it. @wim leers: I hope I didn't miss anything important that you would have caught.
Comment #17
wim leers@effulgentsia This MR had many more commits after my December 19 review than before. This is one of the bigger MRs merged into XB! So to remain on top of this project, I had to review it in its entirety, to understand all changes.
Manual test
CI is passing, but I can't get this to work … 🙈 — when I manually test this, I get this without the checkbox at— a reinstall fixed it, even though it was a fresh install. 👍 Not sure what happened there./admin/appearance/settings/oliverochecked:At minimum, this needs a screenshot illustrating what is newly possible, preferably a screencast. Is the attached GIF a correct illustration?
If it is accurate (please remove the tag then!), then there's a clear bug present: placed blocks that do not render anything in the current context (most obvious one: the "Messages" block) take up vertical space, making the entire preview look off 😅 Tagging for that.
Either way: SO COOL to see this starting to work! 😄😎
Code review
I did do a partial review (ran out of time) of the MR, and would like you to confirm 2 statements to be confirmed to ensure I understand it correctly — and one detail in
PageTemplateDisplayVariantI'm not sure about. 🙏Comment #18
larowlanHere's a screen recording that was shared in community slack during a recent meeting.
I'll add a follow up for the spacing
Comment #19
larowlanAdded #3497744: If an autosave entry exists before enabling global regions for a theme, theme regions cannot be seen which I experienced whilst getting screenshots for the spacing follow up. Perhaps that explains the issue you saw in manual testing Wim?
Opened #3497747: Global regions containing only "dynamically missing blocks" (due to emptiness or access) cause malformed pages for the whitespace - its coming from CSS in Olivero targetting the .region class
Comment #20
wim leers#19: yes, #3497744: If an autosave entry exists before enabling global regions for a theme, theme regions cannot be seen sounds like a highly probable explanation! Thanks for #3497747: Global regions containing only "dynamically missing blocks" (due to emptiness or access) cause malformed pages, too. Bumped both to .
This allowed me to update #3455753 in a detailed way for the status of the
19. Modify the page templatedeliverable: #3455753-32: Milestone 0.2.0: Early preview.