From 821421ed675ca2f5d02bda9e6f8a93a081b68327 Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Thu, 24 Sep 2026 23:22:06 +0100 Subject: [PATCH] fix: Keep Element layout props and convert Element colors validated its props with defaults and applied every prop of its schema on each render, in the order of the engine's setters. The defaults of the props that were not given included the margins and their left, bottom, right and top aliases, which come after height, so an image's height and an element's margins were overwritten on every render: a 200 x 100 image came out 200 x 82, and a stretched element with 20 unit margins came out 412 x 214. The schema was also built from a group element, whose color, outline color and shadow color are null, so colors given as strings or arrays were assigned as they were and became NaN. Apply only the props that are given, as Camera, Collision and Render do, the type first and the anchor, pivot and margins before the width and height, as the engine applies them. Build the schema from a text element so its color props are converted, and let color props also take Color objects and four-component arrays. only allowed "blend", "stretch" and "fit", so the engine's "none" fell back to "blend". Allow "blend" and "none", the two modes the engine has. Co-Authored-By: Claude Opus 5.5 --- .changeset/element-screen-props.md | 5 + packages/lib/src/components/Element.test.tsx | 113 +++++++++++++++++++ packages/lib/src/components/Element.tsx | 31 ++++- packages/lib/src/components/Screen.test.tsx | 19 +++- packages/lib/src/components/Screen.tsx | 12 +- packages/lib/src/utils/validation.ts | 11 +- 6 files changed, 178 insertions(+), 13 deletions(-) create mode 100644 .changeset/element-screen-props.md diff --git a/.changeset/element-screen-props.md b/.changeset/element-screen-props.md new file mode 100644 index 00000000..10ab9e88 --- /dev/null +++ b/.changeset/element-screen-props.md @@ -0,0 +1,5 @@ +--- +"@playcanvas/react": patch +--- + +Fix `` layout and colors, and ``. `` now applies only the props you give it, the type first and the anchor, pivot and margins before the width and height, so the defaults of the props you leave out no longer overwrite an image's height or an element's margins on every render. `color`, `outlineColor` and `shadowColor` accept CSS color strings, arrays and `Color` objects, where strings and arrays used to leave the element's color as NaN. `` accepts the engine's `scaleMode="none"`; `"stretch"` and `"fit"`, which the engine never supported, are removed. diff --git a/packages/lib/src/components/Element.test.tsx b/packages/lib/src/components/Element.test.tsx index fc124205..95dd8d58 100644 --- a/packages/lib/src/components/Element.test.tsx +++ b/packages/lib/src/components/Element.test.tsx @@ -1,5 +1,6 @@ import { render, waitFor } from '@testing-library/react'; import type { Entity as PcEntity } from 'playcanvas'; +import { Color } from 'playcanvas'; import React from 'react'; import { describe, it, expect } from 'vitest'; @@ -32,3 +33,115 @@ describe('Element', () => { expect(ref.current!.element!.text).toBe('Hello, World!'); }); }); + +describe('Element prop application', () => { + // Renders an element inside a 400 x 300 group element on a screen, and returns its entity + const renderInPanel = async (element: React.ReactElement) => { + const ref = React.createRef(); + const result = render( + + + + + + {element} + + + + ); + await waitFor(() => expect(ref.current?.element).toBeTruthy()); + return { entity: ref.current!, ...result }; + }; + + // Regression: the defaults of the props that were not given, among them the margins and + // their left/right/top/bottom aliases, were applied after the width and height on every + // render, which changed the size of the element. + it('keeps the width and height of an element with a point anchor', async () => { + const { entity } = await renderInPanel( + + ); + + expect(entity.element!.calculatedWidth).toBeCloseTo(200); + expect(entity.element!.calculatedHeight).toBeCloseTo(100); + }); + + it('keeps the margins of an element with split anchors', async () => { + const { entity } = await renderInPanel( + + ); + + expect(entity.element!.calculatedWidth).toBeCloseTo(360); + expect(entity.element!.calculatedHeight).toBeCloseTo(260); + }); + + it('keeps the size of an element when it renders again', async () => { + const ref = React.createRef(); + const tree = (label: string) => ( + + + + + + + + + ); + const { rerender } = render(tree('first')); + await waitFor(() => expect(ref.current?.element).toBeTruthy()); + + rerender(tree('second')); + await waitFor(() => expect(ref.current?.name).toBe('second')); + + expect(ref.current!.element!.calculatedWidth).toBeCloseTo(200); + expect(ref.current!.element!.calculatedHeight).toBeCloseTo(100); + }); + + // Regression: the schema was built from a group element, whose color is null, so a color + // given as a string was assigned as it was and the element's color became NaN. + it('converts a color given as a hex string', async () => { + const { entity } = await renderInPanel(); + + const color = entity.element!.color; + expect(color.r).toBeCloseTo(1); + expect(color.g).toBeCloseTo(128 / 255); + expect(color.b).toBeCloseTo(0); + }); + + it('accepts a color given as an array or a Color', async () => { + const { entity: fromArray } = await renderInPanel(); + expect(fromArray.element!.color.g).toBeCloseTo(1); + + const { entity: fromColor } = await renderInPanel(); + expect(fromColor.element!.color.b).toBeCloseTo(1); + }); + + it('converts the outline and shadow colors of a text element', async () => { + const { entity } = await renderInPanel( + + ); + + expect(entity.element!.outlineColor.r).toBeCloseTo(1); + expect(entity.element!.shadowColor.b).toBeCloseTo(1); + expect(entity.element!.shadowColor.a).toBeCloseTo(128 / 255); + }); + + it('applies the type before the props that depend on it', async () => { + const { entity } = await renderInPanel(); + + expect(entity.element!.type).toBe('image'); + expect(entity.element!.color.r).toBeCloseTo(1); + expect(entity.element!.color.g).toBeCloseTo(0); + }); +}); diff --git a/packages/lib/src/components/Element.tsx b/packages/lib/src/components/Element.tsx index fdecbd68..f94471c5 100644 --- a/packages/lib/src/components/Element.tsx +++ b/packages/lib/src/components/Element.tsx @@ -7,7 +7,7 @@ import type { FC } from 'react'; import { useComponent } from '../hooks/index.ts'; import type { PublicProps, Serializable } from '../utils/types-utils.ts'; import type { Schema } from '../utils/validation.ts'; -import { validatePropsWithDefaults, createComponentDefinition, getStaticNullApplication } from '../utils/validation.ts'; +import { validatePropsPartial, createComponentDefinition, getStaticNullApplication } from '../utils/validation.ts'; /** * The Element component renders 2D UI content — text, an image, or a group — on an entity. @@ -25,7 +25,10 @@ import { validatePropsWithDefaults, createComponentDefinition, getStaticNullAppl * */ export const Element: FC = (props) => { - const safeProps = validatePropsWithDefaults(props, componentDefinition); + // Apply only the props that are given. An element's anchor, margins, width and height all + // describe the same rectangle, so applying the defaults of the ones that are not given + // would overwrite the ones that are. + const safeProps = orderProps(validatePropsPartial(props, componentDefinition)); useComponent('element', safeProps, componentDefinition.schema); return null; @@ -33,9 +36,31 @@ export const Element: FC = (props) => { type ElementProps = Partial>>; +// Props that others depend on, in the order they need applying: the type first, so that there +// is an image or a text for the image and text props to apply to, then the anchor and pivot, +// and the margins before the width and height, which is the order the engine applies them in. +const ORDERED_PROPS = ['type', 'anchor', 'pivot', 'margin', 'left', 'bottom', 'right', 'top', 'width', 'height']; + +const orderProps = (props: ElementProps): ElementProps => { + const ordered: Record = {}; + for (const key of ORDERED_PROPS) { + if (key in props) ordered[key] = props[key as keyof ElementProps]; + } + for (const [key, value] of Object.entries(props)) { + if (!(key in ordered)) ordered[key] = value; + } + return ordered as ElementProps; +}; + +// The schema is built from a text element rather than a group, whose color, outline color, +// shadow color, alignment and shadow offset are null, so that colors given as CSS strings or +// arrays are converted rather than assigned as they are. const componentDefinition = createComponentDefinition( 'Element', - () => new Entity('mock-element', getStaticNullApplication()).addComponent('element') as ElementComponent, + () => + new Entity('mock-element', getStaticNullApplication()).addComponent('element', { + type: 'text' + }) as ElementComponent, (component) => (component as ElementComponent).system.destroy(), { apiName: 'ElementComponent' } ); diff --git a/packages/lib/src/components/Screen.test.tsx b/packages/lib/src/components/Screen.test.tsx index 2393d17a..93118a90 100644 --- a/packages/lib/src/components/Screen.test.tsx +++ b/packages/lib/src/components/Screen.test.tsx @@ -33,7 +33,7 @@ describe('Screen', () => { it('should render with custom props', () => { const { container } = renderWithProviders( - + ); expect(container).toBeTruthy(); }); @@ -65,4 +65,21 @@ describe('Screen prop application', () => { expect(screen.referenceResolution.x).toBe(1280); expect(screen.referenceResolution.y).toBe(720); }); + + // Regression: the scaleMode validator only allowed "blend", "stretch" and "fit", so the + // engine's "none" was replaced by the "blend" default. + it('applies a scale mode of none', async () => { + const ref = React.createRef(); + render( + + + + + + ); + + await waitFor(() => expect(ref.current?.screen).toBeTruthy()); + + expect(ref.current!.screen!.scaleMode).toBe('none'); + }); }); diff --git a/packages/lib/src/components/Screen.tsx b/packages/lib/src/components/Screen.tsx index b1553f89..952ab5dd 100644 --- a/packages/lib/src/components/Screen.tsx +++ b/packages/lib/src/components/Screen.tsx @@ -41,10 +41,11 @@ interface ScreenProps extends Partial> */ referenceResolution?: [number, number]; /** - * The scale mode of the screen. + * The scale mode of the screen: `"blend"` scales the screen to fit its reference resolution, + * and `"none"` leaves it unscaled. World-space screens are always unscaled. * @default "blend" */ - scaleMode?: 'blend' | 'stretch' | 'fit'; + scaleMode?: 'blend' | 'none'; } const componentDefinition = createComponentDefinition( @@ -73,10 +74,9 @@ componentDefinition.schema = { } }, scaleMode: { - validate: (value: unknown) => - typeof value === 'string' && ['blend', 'stretch', 'fit'].includes(value as string), - errorMsg: (value: unknown) => - `Invalid value for prop "scaleMode": ${value}. Expected one of: "blend", "stretch", "fit".`, + // The engine's SCALEMODE_BLEND and SCALEMODE_NONE. Any other value makes it fall back to "none". + validate: (value: unknown) => typeof value === 'string' && ['blend', 'none'].includes(value as string), + errorMsg: (value: unknown) => `Invalid value for prop "scaleMode": ${value}. Expected one of: "blend", "none".`, default: 'blend' } } as Schema; diff --git a/packages/lib/src/utils/validation.ts b/packages/lib/src/utils/validation.ts index 81b2918b..98b8fd33 100644 --- a/packages/lib/src/utils/validation.ts +++ b/packages/lib/src/utils/validation.ts @@ -361,13 +361,18 @@ export function createComponentDefinition( // Colors if (value instanceof Color) { schema[key as keyof T] = { - validate: (val) => (Array.isArray(val) && val.length === 3) || typeof val === 'string', + validate: (val) => + val instanceof Color || + (Array.isArray(val) && (val.length === 3 || val.length === 4)) || + typeof val === 'string', default: (value as Color).toString(true), errorMsg: (val: unknown) => `Invalid value for prop "${String(key)}": "${val}". ` + - `Expected a hex like "#FF0000", CSS color name like "red", or an array "[1, 0, 0]").`, + `Expected a hex like "#FF0000", CSS color name like "red", an array "[1, 0, 0]" or a Color).`, apply: (instance, props, key) => { - if (typeof props[key] === 'string') { + if (props[key] instanceof Color) { + (instance[key as keyof InstanceType] as Color) = (props[key] as Color).clone(); + } else if (typeof props[key] === 'string') { const colorString = getColorFromName(props[key] as string) || (props[key] as string); (instance[key as keyof InstanceType] as Color) = new Color().fromString(colorString); } else {