Prevent layout shift as the editor mounts, for editors sized by their content - #1188
Prevent layout shift as the editor mounts, for editors sized by their content#1188sedubois wants to merge 12 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds an opt-in server prerendering path for Lexxy inline editors so the initial paint matches the editor’s eventual content height (reducing layout shift), and teaches the custom element to adopt the prerendered content node on connect.
Changes:
- Add
prerender: trueoption to Rails integrations (editor adapter + tag-helper fallback) to emit a sanitized static.lexxy-editor__contentchild inside<lexxy-editor>. - Update
LexicalEditorElementto adopt an existing direct child.lexxy-editor__contentelement instead of always creating a new one. - Add Ruby + Playwright coverage and a dedicated browser fixture demonstrating height-hugging inline-editor behavior.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/elements/editor.js |
Adopts a server-rendered .lexxy-editor__content node (if present) and applies the interactive attributes on connect. |
lib/lexxy/rich_text_area_tag.rb |
Adds prerender: option to the tag-helper fallback and emits prerendered content when opted in (and no explicit block is given). |
lib/action_text/editor/lexxy_editor.rb |
Adds prerender: support to the Rails editor-adapter path by supplying a block that renders the prerendered content node. |
lib/lexxy/prerender.rb |
Introduces shared server-side prerender helper that sanitizes and builds the static content element. |
lib/lexxy.rb |
Requires the new prerender helper. |
test/helpers/prerender_test.rb |
Verifies prerender output, sanitization, blank handling, and block precedence at the helper layer. |
test/browser/fixtures/prerender.html |
Adds a browser fixture demonstrating “height-hugging” inline editor layout with/without prerender. |
test/browser/tests/editor/prerender_adoption.test.js |
Adds Playwright coverage to ensure following content remains stable and prerendered content is adopted (not duplicated). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sedubois
changed the title
|
Refreshed the description now that #1201 has landed, since that changes what this PR is asking for. #1201 fixes the general case; this is the one configuration it structurally cannot reach. An editor sized by its content sets Also retitled to match #1201's vocabulary — the original title said "stabilize height-hugging inline editors" and never used the term layout shift, which made it read as a different concern than it is. To be clear about priority: we are not blocked. We ship the same idea app-side (published copy as a static placeholder stacked in the same grid cell, dropped by CSS once Lexical's root mounts) and it works. This is offered as the cleaner in-library version — node adoption instead of a stacked placeholder, and no |
`<lexxy-editor>` renders empty and builds its contenteditable a frame after load, so an inline field paints at ~0 height and then jumps to full content height, shifting everything below it. That reflow shows on every cold load, reload, and server re-render. Opt in with `rich_text_area(..., prerender: true)`: the helper renders the value into a `.lexxy-editor__content` element inside `<lexxy-editor>`, and on connect LexicalEditorElement adopts that element instead of creating an empty one. Lexical reconciles its parsed state into the adopted element, so the field has its final height from first paint and the live editor lands at the same height — no shift. Absent the prerendered element the empty-editor path is unchanged, so this is backwards compatible and off by default. Verified against the shipped bundle: without adoption the editor builds a second `.lexxy-editor__content` (the server one is orphaned and the field double-renders); with it there is exactly one element and `setRootElement` reconciles into the server node (it gains `data-lexical-editor`). Follow-up: the general, visible-toolbar case still reflows by the toolbar's height when it mounts; reserving that space server-side is left for a later change.
The prerender option only lived in the TagHelper fallback, which Rails 8.2+ never loads — it registers ActionText::Editor::LexxyEditor instead, so `rich_text_area(..., prerender: true)` silently emitted the option as an HTML attribute and the editor still rendered empty. Emit the prerendered content element from the adapter Tag too, via the block Editor::Tag#render_in already supports; an explicit caller block still wins.
… attributes Two hardenings of the prerendered content element, extracted into a shared Lexxy::Prerender used by both the editor-adapter Tag and the TagHelper fallback: - Sanitize the value with Action Text's display sanitizer (whose allow-list this engine already extends). The attribute path is HTML-escaped and DOMPurify cleans it client-side before it reaches the live DOM, but the prerendered element enters the DOM straight from storage — unsanitized, a stored <script> or onerror handler would execute at first paint. - Emit static markup only: no contenteditable, role or aria attributes until Lexical mounts. Before adoption they would advertise an editability that isn't there yet — keystrokes into the static element are discarded when the editor loads its state from the value attribute — and adoption already dresses the node with those attributes. The browser test now pins that.
Covers both integrations — the ActionText::Editor adapter and the TagHelper fallback — since rich_textarea_tag dispatches to whichever is active and CI runs both legs: off by default, the content element and its content, no interactive attributes until adoption, sanitization of the injected HTML, the blank-value paragraph, and an explicit block winning over the option.
285d517 to
2773949
Compare
|
Rebased onto
The thing I most wanted to check after #1201: the reproduction still reproduces. The fixture's control editor is asserted to move by more than 60px as Lexxy mounts, and that assertion passes against current One gap I'd flag honestly: the fixture exercises |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
test/browser/tests/editor/prerender_adoption.test.js:28
- This assertion only rejects downward movement; a large upward layout shift produces a negative delta and still passes. Since the behavior under test is stability in either direction, assert the absolute displacement.
expect(withAfter - withBefore).toBeLessThan(1)
lib/lexxy/prerender.rb:22
- For a content-sized editor with the default toolbar, this still does not reserve the mounted height. When
--lexxy-editor-rows: auto, the pre-upgrade host reservation is invalid, this child contributes only the body height, and#createDefaultToolbar()later prepends an in-flow toolbar, shifting following content by the toolbar height. The fixture masks this case by making its toolbarposition: absolute. Either support/reserve the in-flow toolbar case or scope and document this option as requiring an out-of-flow toolbar, with a matching test.
view.content_tag "div", html, class: "lexxy-editor__content"
|
Both suppressed comments were worth acting on — the second one especially. Addressed in f8a19a1. 1. Signed vs absolute displacement ( 2. In-flow default toolbar ( Exactly the toolbar's height. I took the "scope and document, with a matching test" option:
The reasoning for scoping rather than reserving: the editors this option is for float the toolbar out of flow, because an inline field standing in for published copy has nowhere to put one — our own app included. But that is a judgement about the use case, not a technical obstacle. If you'd rather Full chromium suite after the change: 625 collected, 622 passed, 2 flaky on retry (varying, pre-existing), 1 conditional skip. |
Two review points from Copilot on the rebased branch. The stability assertion compared a signed delta against 1, so a large upward shift would have passed as a comfortably negative number. Compare absolute displacement. The second is substantive and correct. Prerendering reserves the content element's height, but #createDefaultToolbar prepends the default toolbar in normal flow during connectedCallback, so an editor using it still gains that height on mount — measured at 41px, exactly the toolbar's height, once the fixture's out-of-flow positioning is removed. The fixture floats its toolbar, so nothing exercised this. Scope it explicitly rather than leave it implied: document what the option does and does not reserve, and add a test that puts the toolbar back in flow and pins the residual shift to the toolbar's height. The editors this option is for float the toolbar out of flow — an inline field standing in for published copy has nowhere to put one — but a reader should not have to infer that from the fixture's CSS.
…itor Prerendering reserved the content element's height but not the toolbar's, so an editor using the default in-flow toolbar still moved the page by exactly that element's height on mount — 41px, measured with the fixture's out-of-flow positioning removed. The option is meant to mean "this editor will not shift", so make it true rather than document the gap. Ship the toolbar itself, empty, ahead of the content element, and fill it in on connect instead of building a second one. It has to be a real <lexxy-toolbar> rather than a neutral spacer: whether a toolbar takes space at all is the host's CSS to decide, and an inline editor floats it out of flow. A spacer sized from --lexxy-toolbar-height would reserve height nothing fills there and shift the page *up* — which is exactly what the first attempt did, caught by the existing stability test. Two details worth keeping: - The reservation is a border-box one. --lexxy-toolbar-height already counts the toolbar's padding and border, and the toolbar is content-box, so applying it as a plain min-block-size added both a second time and left 5px to give back. - Only reserve a toolbar the editor is actually going to prepend: rich text off, or a `toolbar` naming an element by id, means none arrives, and reserving then is the same defect upwards. The client drops a prerendered toolbar it turns out not to want, in the same task as connect, so nothing paints in between. Tests: the fixture keeps its floated toolbar, so the existing stability test now covers "reserves nothing when the toolbar takes no space"; a new one puts the toolbar back in normal flow and requires the same stability; a third pins that the prerendered toolbar is filled rather than duplicated. Ruby-side, the emitted toolbar and each of the three opt-out conditions.
f8a19a1 to
99e221e
Compare
|
Implemented the in-flow toolbar reservation in 99e221e, so The editor ships its toolbar too, empty, ahead of the content element, and fills it in on connect instead of building a second one. It has to be a real Two details that took measuring:
Tests: the fixture keeps its floated toolbar, so the original stability test now also covers "reserves nothing when the toolbar takes no space"; a new one puts the toolbar back in normal flow and requires the same stability; a third pins that the prerendered toolbar is filled rather than duplicated. Ruby-side, the emitted toolbar and each opt-out condition. Full chromium suite 622 passed / 2 flaky (pre-existing, varying), Separately — apologies for the noise in the previous push: it briefly added a |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
lib/lexxy/prerender.rb:56
lexxy_rich_textarea_tagsymbolically normalizes its options, so the valid"rich-text": falseoption arrives here as:"rich-text". This lookup misses that key, prerenders a toolbar, and the client then removes it because rich text is disabled, reintroducing an upward layout shift. The current test usesrich_text: false, which does not produce therich-textattribute the client reads; please cover the hyphenated option too.
rich_text = options.fetch(:rich_text) { options.fetch("rich-text") { options["rich_text"] } }
app/assets/stylesheets/lexxy-editor.css:428
- The reservation is intended only for the newly generated placeholder, but this selector applies it to every unupgraded toolbar. A pre-existing custom child toolbar can therefore be forced to the default toolbar height before upgrade and shrink when
:definedstarts matching, introducing CLS for editors that did not opt into prerendering. Scope the rule to thedata-prerenderedmarker emitted bytoolbar_placeholder_tag.
:where(lexxy-toolbar:not(:defined)) {
lib/lexxy/prerender.rb:62
- This decision cannot actually mirror the client when toolbar/rich-text behavior comes from the documented
Lexxy.configuredefaults or a named preset. For example, an editor using a preset withrichText: falsehas no corresponding helper option here, so the server reserves a toolbar and#discardUnusedPrerenderedToolbarremoves it on connect, causing the shift this option is meant to prevent. The API needs a reliable server-side signal for whether to reserve the toolbar (or this limitation must be avoided by not reserving it implicitly), with a browser test for configured presets/defaults.
return false if [ false, "false" ].include?(rich_text)
return false if [ false, "false" ].include?(toolbar)
return false if toolbar.is_a?(String)
true
Problem
Loading a page whose editor is sized by its content shows a layout shift as the editor mounts: the field paints at ~0px and jumps to its content height, shoving everything below it down the page.
This is the same symptom #1201 fixed for the default editor, in the one configuration that fix cannot reach.
Why #1201 doesn't cover this
#1201 reserves the mounted height on the host before upgrade:
That works whenever
--lexxy-editor-rowsis a length — including a per-instance override like the sandbox's30lh. It cannot work for an editor that is sized by its content, for two independent reasons:The reservation evaluates to nothing. A content-sized field sets
--lexxy-editor-rows: auto, andcalc(30px + auto + 0px)is not a valid length, so the whole declaration is dropped andmin-block-sizestays0. Verified in Chrome 150: with--lexxy-editor-rows: autothe computedmin-block-sizeis0px, where8lhgives246px.A row count is the wrong height anyway. These editors stand in for published copy in the page around them, so the height to reserve is this record's rendered height — 106px for one body, 35px for another — not N rows. Reserving a fixed number of rows would replace a shift down with a shift up.
Setting
--lexxy-editor-rows: 0(as an earlier revision of this PR suggested) does not rescue it either: the reservation then collapses to the toolbar height, which for these editors is out of flow.Measured on a real app, with the custom-element registration deferred so the pre-upgrade window is observable:
--lexxy-editor-rows: 8lh)--lexxy-editor-rows: auto)Fix
Opt-in prerendering, so the reserved height is the record's own:
Rails renders the editor's current value as a static content element inside
<lexxy-editor>:It also ships the toolbar itself, empty, ahead of that content element, and fills it in on connect rather than building a second one — so the reservation covers the whole mounted height, not just the body. That has to be a real
<lexxy-toolbar>rather than a neutral spacer: whether a toolbar occupies space at all is the host's CSS to decide, and an inline editor floats it out of flow, where a spacer would reserve height nothing fills and shift the page up. No toolbar is emitted when the editor will not prepend one (rich text off, or atoolbarnaming an element by id).When Lexxy connects it adopts that existing
.lexxy-editor__contentinstead of appending an empty one. Lexical still parsesvalueas the source of truth and reconciles its state into the adopted node; the prerendered child only gives the browser the right layout before JavaScript finishes.Off by default. Without
prerender: true, and when callers pass an explicit block, the markup path is unchanged. It composes with #1201 rather than competing: #1201 reserves a height for editors that have one to reserve, this reserves this field's height for editors that do not.Where the empty-editor window shows up
Cold load and reload, back/forward without a Turbo cache hit, server re-rendered forms, Turbo stream refreshes that morph the editor back toward server HTML, and any slow connection or heavy page — where the window lasts longest and the shift is most visible.
Reproduction
A browser fixture in this branch demonstrates the case:
Then open
http://localhost:5173/prerender.html?delay=1500. Two editors with identical content and content below them: the left has no prerendered child and the content below it jumps when delayed initialization runs; the right is prerendered and the content below it starts and stays put. The browser test pins this by measuring the following content before and after Lexxy connects.prerender-layout-shift.mp4
Design notes
valueremains the source of truth. Adoption reuses the DOM node; it does not trust the child markup as state.contenteditable,roleor ARIA — until Lexxy adopts it, so the page does not advertise an editor that cannot yet accept input.Lexxy::Prerender.Known limitations
valueand as a child element — only when opted in.Status
Not blocking us. We ship the same idea app-side: the published copy is rendered as a static placeholder stacked in the same CSS grid cell as the editor, and CSS drops it once Lexical's mounted root appears. That works and is tested, so this PR is offered as the cleaner in-library version — adoption instead of a stacked placeholder, no
:has()handoff rule for consumers to get right — not as something we need merged.One gotcha worth recording either way, in case it informs the design: the handoff must key on Lexical's mounted root (
[data-lexical-editor]), not on Lexxy's ownconnectedattribute.connectedis set at the end ofconnectedCallbackbut the root mounts a frame later, so handing off onconnectedcollapses the field for a frame — the same layout shift, smaller. Adoption avoids the question entirely, which is one of the reasons this belongs in the library.Testing
Rebased onto
mainat 41f776e (0.9.28 + #1201), conflict-free, and re-verified there:yarn lint— clean.yarn test— 128 unit tests, 19 files, all passing.paste/plain_text_paths,tables/toolbar,editor/leak) and are pre-existing and unrelated.test/browser/tests/editor/prerender_adoption.test.js— 5 passing: the reproduction, stability with the toolbar floated and in normal flow, and that the prerendered toolbar is filled rather than duplicated.test/helpers/prerender_test.rb— 6 runs, 19 assertions:rich_textarea_tagoutput, sanitization, blank values, and block precedence.Worth noting for review: the reproduction still reproduces on top of #1201. The fixture's control editor is asserted to move by more than 60px as Lexxy mounts, and that assertion passes against current
main— because with--lexxy-editor-rows: 0the new reservation collapses to the toolbar height, which in this layout is out of flow. So the two changes do not overlap in practice.