From 5dde693a3bb7f15e1df78c0bd826ffe15c77e9d2 Mon Sep 17 00:00:00 2001 From: Douwe Bos Date: Wed, 23 Sep 2026 16:06:29 +0200 Subject: [PATCH] fix(flexbox): keep a hidden node at the position it was laid out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yoga zeroes the computed box of a display:none node. Emitting that layout dragged the node and its subtree to the origin, so an element parked off screen by a translate read as on screen to anything walking the scene graph — the Plex TV drawer's nav items showed up in the testID mirror at 0,0 while the drawer was closed. Populates the _hiddenElements set _getUpdatedStyles already consults, so a hidden node stops emitting updates until it is shown again. --- .changeset/hidden-node-keeps-position.md | 5 ++ .../src/YogaManager.displayToggle.test.ts | 65 ++++++++++++++++++- .../plugin-flexbox/src/YogaManager.spec.ts | 37 +++++++++-- packages/plugin-flexbox/src/YogaManager.ts | 22 +++++-- 4 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 .changeset/hidden-node-keeps-position.md diff --git a/.changeset/hidden-node-keeps-position.md b/.changeset/hidden-node-keeps-position.md new file mode 100644 index 0000000..9aaa859 --- /dev/null +++ b/.changeset/hidden-node-keeps-position.md @@ -0,0 +1,5 @@ +--- +"@plextv/react-lightning-plugin-flexbox": patch +--- + +A `display: 'none'` node keeps the position it was laid out at instead of being moved to the origin. Yoga zeroes the computed box of a hidden node, and emitting that dragged the node — and its subtree — to 0,0, so an element parked off screen by a translate read as on screen to anything walking the scene graph. diff --git a/packages/plugin-flexbox/src/YogaManager.displayToggle.test.ts b/packages/plugin-flexbox/src/YogaManager.displayToggle.test.ts index 294de04..94d54c0 100644 --- a/packages/plugin-flexbox/src/YogaManager.displayToggle.test.ts +++ b/packages/plugin-flexbox/src/YogaManager.displayToggle.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from 'vitest'; -import { YogaManager } from './YogaManager'; + import { SimpleDataView } from './util/SimpleDataView'; +import { YogaManager } from './YogaManager'; function decode(buffer: ArrayBuffer) { const view = new SimpleDataView(buffer); - const out: Record = - {}; + const out: Record = {}; while (view.offset < buffer.byteLength) { const id = view.readUint32(); @@ -60,6 +60,38 @@ async function setup() { return { m, screenStyle, layout: () => layout }; } +// A drawer parked off-screen by a translate, then hidden once it has finished +// closing — the position must survive the hide. +async function setupTranslatedOffscreen() { + const m = new YogaManager(); + await m.init(); + + m.addNode(1); + m.applyStyle(1, { display: 'flex', w: 800, h: 1000 }, true); + m.addIndependentRoot(1); + + const drawerStyle = { + position: 'absolute', + top: 0, + left: 0, + w: 452, + h: 1000, + display: 'flex', + transform: { translateX: -452 }, + } as const; + + m.addNode(2); + m.applyStyle(2, { ...drawerStyle }, true, true); + m.addChildNode(1, 2); + + let layout: ReturnType = {}; + m.on('render', (buf) => { + layout = { ...layout, ...decode(buf) }; + }); + + return { m, drawerStyle, layout: () => layout }; +} + describe('display none -> flex round trip', () => { it('keeps default column direction after toggling display', async () => { const { m, screenStyle, layout } = await setup(); @@ -90,4 +122,31 @@ describe('display none -> flex round trip', () => { expect(layout()[4]).toMatchObject({ x: 0, y: 112 }); }); + + it('leaves a hidden node where it was laid out', async () => { + const { m, drawerStyle, layout } = await setupTranslatedOffscreen(); + + m.flushLayout(); + expect(layout()[2]).toMatchObject({ x: -452, y: 0 }); + + // Yoga zeroes the computed box of a display:none node. Emitting that would + // drag the hidden subtree back to the origin, where anything reading the + // scene graph sees it as on screen. + m.applyStyle(2, { ...drawerStyle, display: 'none' }, true, true); + m.flushLayout(); + + expect(layout()[2]).toMatchObject({ x: -452, y: 0 }); + }); + + it('lays a hidden node back out when it is shown again', async () => { + const { m, drawerStyle, layout } = await setupTranslatedOffscreen(); + + m.applyStyle(2, { ...drawerStyle, display: 'none' }, true, true); + m.flushLayout(); + + m.applyStyle(2, { ...drawerStyle, display: 'flex', transform: { translateX: 0 } }, true, true); + m.flushLayout(); + + expect(layout()[2]).toMatchObject({ x: 0, y: 0, w: 452 }); + }); }); diff --git a/packages/plugin-flexbox/src/YogaManager.spec.ts b/packages/plugin-flexbox/src/YogaManager.spec.ts index a99a525..e271b2d 100644 --- a/packages/plugin-flexbox/src/YogaManager.spec.ts +++ b/packages/plugin-flexbox/src/YogaManager.spec.ts @@ -22,6 +22,7 @@ const mockNode = { getMaxWidth: vi.fn(), // Unanchored edge (unit UNDEFINED), matching a node with no right/bottom set. getPosition: vi.fn(() => ({ unit: 0, value: undefined })), + getDisplay: vi.fn(() => 0), getParent: vi.fn(), markLayoutSeen: vi.fn(), }; @@ -58,6 +59,8 @@ const mockYoga = { ERRATA_STRETCH_FLEX_BASIS: 3, ERRATA_ABSOLUTE_PERCENT_AGAINST_INNER_SIZE: 4, ERRATA_ABSOLUTE_POSITION_WITHOUT_INSETS_EXCLUDES_PADDING: 5, + DISPLAY_FLEX: 0, + DISPLAY_NONE: 1, }; vi.mock('yoga-layout/load', () => ({ @@ -557,8 +560,17 @@ describe('YogaManager', () => { yogaManager.addNode(elementId); // Closed: translateX shifts the node off-screen to the left. - yogaManager.applyStyle(elementId, { x: 0, transform: { translateX: -452 } }); - expect(applyFlexPropToYoga).toHaveBeenCalledWith(mockYoga, mockYogaOptions, mockNode, 'left', -452); + yogaManager.applyStyle(elementId, { + x: 0, + transform: { translateX: -452 }, + }); + expect(applyFlexPropToYoga).toHaveBeenCalledWith( + mockYoga, + mockYogaOptions, + mockNode, + 'left', + -452, + ); vi.mocked(applyFlexPropToYoga).mockClear(); @@ -567,7 +579,13 @@ describe('YogaManager', () => { // holds even for a partial (resetMissing=false) push — the transform key is // authoritative for its own axes — so the -452 inset must clear, not stick. yogaManager.applyStyle(elementId, { x: 0, transform: {} }, false, false); - expect(applyFlexPropToYoga).toHaveBeenCalledWith(mockYoga, mockYogaOptions, mockNode, 'left', 0); + expect(applyFlexPropToYoga).toHaveBeenCalledWith( + mockYoga, + mockYogaOptions, + mockNode, + 'left', + 0, + ); }); it('keeps a translate inset when the push omits transform entirely', async () => { @@ -575,14 +593,23 @@ describe('YogaManager', () => { const elementId = 654; yogaManager.addNode(elementId); - yogaManager.applyStyle(elementId, { x: 0, transform: { translateX: -452 } }); + yogaManager.applyStyle(elementId, { + x: 0, + transform: { translateX: -452 }, + }); vi.mocked(applyFlexPropToYoga).mockClear(); // No transform key in this push: the translate is not part of the update // and must be left alone, not reset to 0. yogaManager.applyStyle(elementId, { w: 100 }, false, false); - expect(applyFlexPropToYoga).not.toHaveBeenCalledWith(mockYoga, mockYogaOptions, mockNode, 'left', 0); + expect(applyFlexPropToYoga).not.toHaveBeenCalledWith( + mockYoga, + mockYogaOptions, + mockNode, + 'left', + 0, + ); }); it('should apply multiple styles', async () => { diff --git a/packages/plugin-flexbox/src/YogaManager.ts b/packages/plugin-flexbox/src/YogaManager.ts index 337d1ad..dc9c481 100644 --- a/packages/plugin-flexbox/src/YogaManager.ts +++ b/packages/plugin-flexbox/src/YogaManager.ts @@ -8,10 +8,7 @@ import { layoutText, type TextMeasureProps } from './text/layoutText'; import type { ManagerNode } from './types/ManagerNode'; import type { YogaOptions } from './types/YogaOptions'; import applyReactPropsToYoga, { applyFlexPropToYoga } from './util/applyReactPropsToYoga'; -import { - resolveHorizontalTranslate, - resolveVerticalTranslate, -} from './util/resolveTranslateInset'; +import { resolveHorizontalTranslate, resolveVerticalTranslate } from './util/resolveTranslateInset'; import { SimpleDataView } from './util/SimpleDataView'; export type BatchedUpdate = Record>; @@ -248,6 +245,7 @@ export class YogaManager { public removeNode(elementId: number): void { this._textNodes.delete(elementId); + this._hiddenElements.delete(elementId); const yogaNode = this._elementMap.get(elementId); @@ -451,7 +449,12 @@ export class YogaManager { const style = styles[elementId as unknown as number]; if (style !== undefined) { - this.applyStyle(+elementId, style, skipRender, resets?.[elementId as unknown as number] === 1); + this.applyStyle( + +elementId, + style, + skipRender, + resets?.[elementId as unknown as number] === 1, + ); } } } @@ -480,6 +483,15 @@ export class YogaManager { applyReactPropsToYoga(this._yoga, this._yogaOptions, yogaNode, style, resetMissing); + // Yoga zeroes the computed box of a display:none node, so emitting its + // layout would drag the node back to the origin — a node parked off screen + // by a translate lands in view of anything reading the scene graph. + if (yogaNode.node.getDisplay() === this._yoga.DISPLAY_NONE) { + this._hiddenElements.add(elementId); + } else { + this._hiddenElements.delete(elementId); + } + if (style.transform) { const { x, y, transform } = style;