Lay out a scroll container's content when the content changes
A short list opened after a long one drew all its rows stacked on one point. The fault was not in the list tab: rebuild() handed the row stack to setContent() while it was still empty, so the content was sized to max(rows, viewport) = the viewport. The rows arrived after that, and for a short list the same formula yields the same height, so resize() saw no change and skipped layout(). A long list changes the height and never showed it. relayoutContent() now runs whenever the content changed, rather than leaning on resize()'s change detection - that detection is the wrong instrument for "the children moved". The list tab was the only affected caller: the panels use ListView, which positions its rows itself, and the inspector fills its body before setContent. Also drops the comma line under the list. Every section that can carry a list supplies openList, so the comma fallback in fieldFactory was unreachable; listToText/listFromText go with it. The list-value test now covers coerceListItem through coerceValue instead, so the boolean spelling stays guarded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e52e95c0e9
commit
4d07946c74
9 files changed
+102
-87
No files matched your search
@@ -4,7 +4,7 @@ import { SetOverrideCommand } from '../../src/core/commands/sceneObjectCommands'
|
||||
import { sameValue, textOf } from '../../src/core/model/Properties';
|
||||
import { findAsset, objectHolders, type Project } from '../../src/core/model/Project';
|
||||
import { deserializeProject } from '../../src/core/store/serialize';
|
||||
import { coerceValue, listFromText, listToText, variableDef } from '../../src/core/properties/PropertySchema';
|
||||
import { coerceValue, variableDef } from '../../src/core/properties/PropertySchema';
|
||||
import { deviatesFromAsset } from '../../src/core/properties/resolve';
|
||||
import { asEntity, asStruct, writeTyped } from '../../src/core/graph';
|
||||
import { fakeEntity, makeBehavior, makeSpriteScene, place, roundTrip } from '../helpers';
|
||||
@@ -78,15 +78,19 @@ describe('a list as a value', () => {
|
||||
expect(deviatesFromAsset(placed, asset, 'row')).toBe(true);
|
||||
});
|
||||
|
||||
it('reads and writes as one comma-separated line, item kind by item kind', () => {
|
||||
expect(listFromText('1, 2, 3', 'number')).toEqual([1, 2, 3]);
|
||||
expect(listFromText('', 'number')).toEqual([]);
|
||||
expect(listFromText('a, b', 'text')).toEqual(['a', 'b']);
|
||||
it('coerces a loaded item to its declared kind, kind by kind', () => {
|
||||
expect(coerceValue(variableDef('n', 'list', [], 'number'), ['1', 2, 'x'])).toEqual([1, 2, 0]);
|
||||
expect(coerceValue(variableDef('t', 'list', [], 'text'), [1, true])).toEqual(['1', 'true']);
|
||||
// Spelling has to count here: Boolean('false') would be true.
|
||||
expect(listFromText('true, false, 0', 'boolean')).toEqual([true, false, false]);
|
||||
expect(listFromText('#ff0000, #00ff00', 'color')).toEqual([0xff0000, 0x00ff00]);
|
||||
expect(listToText([0xff0000], 'color')).toBe('#ff0000');
|
||||
expect(listToText([1, 2], 'number')).toBe('1, 2');
|
||||
expect(coerceValue(variableDef('b', 'list', [], 'boolean'), ['true', 'false', '0', ''])).toEqual([
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
]);
|
||||
expect(coerceValue(variableDef('c', 'list', [], 'color'), ['#ff0000', 0x00ff00])).toEqual([
|
||||
0xff0000, 0x00ff00,
|
||||
]);
|
||||
});
|
||||
|
||||
it('a value that is not a list falls back to the default rather than becoming one', () => {
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { ScrollContainer, Spacer, VStack } from '../../src/ui';
|
||||
|
||||
/** Row tops after a layout; an unlaid-out stack leaves every child at 0. */
|
||||
const tops = (stack: VStack) => stack.children.map((c) => c.y);
|
||||
|
||||
describe('a scroll container told its content changed', () => {
|
||||
it('lays out rows that leave the height pinned to the viewport', () => {
|
||||
const scroll = new ScrollContainer();
|
||||
scroll.resize(200, 300);
|
||||
const rows = new VStack();
|
||||
// The list tab hands over the stack before filling it, so the empty content is measured first
|
||||
// and pinned to the viewport height.
|
||||
scroll.setContent(rows);
|
||||
rows.add(new Spacer(20));
|
||||
rows.add(new Spacer(20));
|
||||
rows.add(new Spacer(20));
|
||||
|
||||
// 60 < 300, so the content keeps the height it already had: `resize` alone would see no
|
||||
// change and skip the layout, leaving all three rows stacked at the top.
|
||||
scroll.relayoutContent();
|
||||
expect(tops(rows)).toEqual([0, 20, 40]);
|
||||
});
|
||||
|
||||
it('still lays out rows that make the content taller than the viewport', () => {
|
||||
const scroll = new ScrollContainer();
|
||||
scroll.resize(200, 50);
|
||||
const rows = new VStack();
|
||||
scroll.setContent(rows);
|
||||
rows.add(new Spacer(20));
|
||||
rows.add(new Spacer(20));
|
||||
rows.add(new Spacer(20));
|
||||
|
||||
scroll.relayoutContent();
|
||||
expect(tops(rows)).toEqual([0, 20, 40]);
|
||||
expect(scroll.maxScroll).toBe(10);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user