Problem/Motivation

Since #3629497: Render the Canvas in an iframe on the front theme the Canvas renders on the front theme, in an iframe, with the theme's scripts. The Preview beside it, in the split view, now mostly shows the same render at half the width. It still gives three things the Canvas does not: the draft on its pinned page with the theme's regions around it, the output without dropzones, boxes and placeholders, and working links and forms, which the Canvas blocks to select nodes. Visual builders whose editing surface is the real render preview in place, Webflow and Webstudio with a toggle, and the WordPress block editor adds a "Preview in new tab" link. A split view fits tools whose editor is a form, as Sanity's Presentation tool.

Proposed resolution

Replace the split view with a preview mode of the Canvas pane. A toggle in the Canvas header, also on the p shortcut, shows the preview page in a second iframe of the Canvas, with the same viewport and zoom, loaded on the first switch and refreshed only while shown. Editing is off while it shows, and switching back keeps the selection. Next to the toggle, an "Open in new tab" link opens the same page, kept live: the builder page posts on a BroadcastChannel when the preview token changes, and the tab reloads with its scroll kept. The preview island keeps its ID and becomes this control, a pane header island attached to the Canvas, so profiles need no migration, and the Preview island type goes.

Remaining tasks

  • Preview mode in the Canvas pane: second iframe, refresh while shown, editing and libraries off, selection kept.
  • The preview island as the Canvas header toggle and new tab link, p kept.
  • Live new tab through BroadcastChannel, scroll kept.
  • Chrome indicator in the Canvas header, shown in preview mode.
  • Remove the split view, the Preview pane and IslandType::Preview.
  • Tests, change record.

User interface changes

No split view. The Canvas header gets a preview toggle and an "Open in new tab" link, and p toggles the preview mode instead of the split.

API changes

IslandType::Preview is removed, and the preview island becomes a Floating island attached to the Canvas.

Data model changes

None on the server: the island keeps its ID. The split view's browser storage, splitView, splitRatio and splitCollapsed, is no longer read.

AI Generated: Yes

CommentFileSizeAuthor
#11 dark.png27.32 KBpdureau
#11 light.png26.72 KBpdureau

Issue fork display_builder-3630345

Command icon 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:

About issue forks

Comments

mogtofu33 created an issue. See original summary.

pdureau’s picture

pdureau commented

It is amazing:

  • Simpler and more sensible UX (i forgot to report it in the Rotterdam demos feedback, but some people were confused by the Preview mechanism, and I am sure they will prefer this one)
  • Simpler implementation: one less island type, a single rendering strategy (iframe only, no DSD)

It works well:

UI Suite
DaisyUI 		UI Suite
Bootstrap 		UI Suite
DSFR
Component rendering & prop updates ✅ ✅ Not tested
Block rendering & settings updates ✅ ✅ Not tested
Style utilities updates ✅ ✅ Not tested
Design tokens updates & inheritance ✅ BS5 tokens are a mess No tokens in DSFR
Component library preview popin ✅ ✅ Not tested
Block library preview pop-in ✅ ✅ Not tested
Patterns library preview pop-in ✅ Not tested
UI Skins theme switch ✅ ✅ Not tested
Carousels & Accordions Not applicable ✅ Not tested

There are some little issues:

  • z-index of the draggable line hover the contextual panel (but maybe it was already like in #3629497: Render the Canvas in an iframe on the front theme)
  • fixed widths of library preview pop-in (the price" to pay to move from DSD to iframe, I guess)
  • the "open in a new tab" features, it is cool (with the live update!) but I am afraid it may be confusing, especially for display without full page rendering

But nothing serious. My only warning: I feel a little slowness during usage compared to the current 1.0.x. No metric. Maybe it is the current state of my local environment...

I will check again when the MR will be ready.

mogtofu33’s picture

mogtofu33 commented
Status: Active » Needs work

Yes there is currently performance issues, I am fixing some here and other on a follow up.
A big improvement for some cases is from #3630451: Source labels and prop source lists redo the full lookup on each call and #3630456: SourceValueItem::setValue() walks the whole source tree through its schema on each load.

mogtofu33’s picture

mogtofu33 commented
Parent issue: #3562060: Render UI with admin theme » #3629497: Render the Canvas in an iframe on the front theme
Related issues: -#3629497: Render the Canvas in an iframe on the front theme +#3630611: Viewport switcher: fit zoom, visible width, a phone width

mogtofu33 opened merge request !473

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Needs work » Needs review
pdureau’s picture

pdureau commented

Yes there is currently performance issues, I am fixing some here and other on a follow up.

Currently reviewing again, now the MR is created, but I don't see any new commit since yesterday morning.

mogtofu33’s picture

mogtofu33 commented

As discussed, performance fixes were on 2 other issue merged and rebased here:

mogtofu33’s picture

mogtofu33 commented
Related issues: +#3630489: Instance history rewrites every revision to the entity cache after each change, +#3630609: API controllers save the instance a second time after each change
mogtofu33’s picture

mogtofu33 commented

And for the UI Patterns patches, they are pretty relevant on a local automated benchmark:

Config 595, labels 596, node IDs
A off off
B on off
C off on
D on on

Server, median ms

Instance Request A B C D D vs A
Article, 104 nodes builder 1964 1201 1256 471 -76%
canvas 600 212 589 203 -66%
select 275 228 260 213 -23%
style 317 264 254 204 -36%
Override, 89 nodes builder 1853 1208 1143 484 -74%
canvas 568 234 552 203 -64%
select 466 473 442 452 -3%
style 495 499 434 444 -10%
Page layout, 135 nodes builder 983 984 274 274 -72%
canvas 150 150 133 130 -13%
select 327 260 309 239 -27%
style 381 309 308 235 -38%
Views, 59 nodes builder 982 987 275 275 -72%
canvas 162 162 158 158 -2%
select 226 204 217 194 -14%
style 244 221 209 185 -24%
Test page, 2694 nodes builder 2282 2114 1578 1398 -39%
canvas 1629 1527 1189 1086 -33%
select 625 615 205 193 -69%
style 2513 2523 368 361 -86%

Browser, median ms, click to builder idle

Instance Step A B C D D vs A
Article load 2555 1832 1792 1017 -60%
select 454 398 426 382 -16%
style 738 438 664 368 -50%
Override load 2603 2032 1803 1240 -52%
select 573 569 547 540 -6%
style 836 630 766 553 -34%
Page layout load 1732 1690 973 888 -49%
select 713 640 687 610 -14%
style 504 456 432 357 -29%
Views load 1596 1619 843 830 -48%
select 385 367 381 362 -6%
style 359 346 348 311 -13%
Test page load 7489 7258 5969 5615 -25%
select 1700 1692 1317 1254 -26%
style 5793 5680 3153 3037 -48%

Test page style, A column: A2. The first A run started with a shorter history and measured 5030 ms; see the history finding below.

Call counts, xhprof

Taken for D, C, and A on the article and the override builder page.

Request Function Without With
Article canvas ContextDefinition::isSatisfiedBy() 32,882 3,463
Article builder ContextDefinition::isSatisfiedBy() 98,048 37,056
Override canvas ContextDefinition::isSatisfiedBy() 29,750 4,040
Page layout select ContextDefinition::isSatisfiedBy() 37,156 15,032
Test builder getChoices() 1,621 1
Test canvas getChoices() 810 0
Article builder typed config created by ensureNodeIds() 71 0
Article style typed config created by ensureNodeIds() 17 1

The first four rows compare C with D, the effect of !595. The last two compare A with C, the effect of !596.

Verdict

  • !595 helps most where nodes use entity field sources: the article and the override. Canvas -60 to -65%, builder -35 to -39%. Page layouts and Views: Canvas
    unchanged, select and style -10 to -20%. Test page -6%.
  • !596 cuts the builder page by 31 to 72% everywhere, and a change by 12 to 20% on normal pages, 85% on the test page. The builder page gain comes from loading
    every instance, see below.
  • Both add up, as they target different costs. The article Canvas drops from 600 to 203 ms, a style change from 317 to 204 ms. Felt on DDEV: the article loads
    in 1.0 s instead of 2.6 s, a style change takes 368 ms instead of 738 ms.
  • No regression measured. The +1 to 2% of B on the override is noise, inside the p90.
pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new light.png26.72 KB
new dark.png27.32 KB

Performance

Thanks for the exhaustive metrics.

However, I don't need those to move this ticket to RTBC: The perf is now good enough on my local environment, maybe thanks to #3630609: API controllers save the instance a second time after each change & #3630489: Instance history rewrites every revision to the entity cache after each change

Previous feedback

The other feedback are OK or not blocking:

  • fixed widths of library preview pop-in : not a big deal, I am not even sure we need to create a follow-up ticket
  • z-index of the draggable line hover the contextual panel: I am not reproducing
  • the "open in a new tab" features stays: let's wait users feedback before discussing it

Except light / dark mode

Emptying the local storage confirm it is OK when both have the same config light / dark / "system/auto".

But the placeholders are not readable

Light mode Dark mode
light light

Is it somethign to address here before merging? Or in a dedicated ticket ?

  • mogtofu33 committed 13c7d18e on 1.0.x 
    feat: #3630345 Preview as a switch on the Canvas, and in a new tab,...
mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

pdureau’s picture

pdureau commented
Status: Fixed » Closed (fixed)