Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/hidden-node-keeps-position.md
Original file line number Diff line number Diff line change
@@ -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.
65 changes: 62 additions & 3 deletions packages/plugin-flexbox/src/YogaManager.displayToggle.test.ts
Original file line number Diff line number Diff line change
@@ -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<number, { x: number; y: number; w: number; h: number }> =
{};
const out: Record<number, { x: number; y: number; w: number; h: number }> = {};

while (view.offset < buffer.byteLength) {
const id = view.readUint32();
Expand Down Expand Up @@ -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<typeof decode> = {};
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();
Expand Down Expand Up @@ -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 });
});
});
37 changes: 32 additions & 5 deletions packages/plugin-flexbox/src/YogaManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -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();

Expand All @@ -567,22 +579,37 @@ 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 () => {
const { applyFlexPropToYoga } = await import('./util/applyReactPropsToYoga');
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 () => {
Expand Down
22 changes: 17 additions & 5 deletions packages/plugin-flexbox/src/YogaManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number, Partial<Rect>>;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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,
);
}
}
}
Expand Down Expand Up @@ -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;

Expand Down
Loading