Skip to content

Commit e7dd936

Browse files
committed
fix(animate): prevent clear-state layout collapse
Clear-state transitions may animate removed state keys back to visual defaults. Those values remain transient animation targets, not static truth. Optional rect geometry aliases stay absent when base/final layout omits them. Natural animation closeout now avoids a second cleanup restore over newer state changes. Constraint: D3 static truth remains baseAttributes plus resolvedStatePatch. Rejected: Restore normalAttrs snapshots | would regress the D3 state model. Rejected: Commit removed state defaults | would pollute finalAttribute. Confidence: high Scope-risk: moderate Directive: Do not default missing optional geometry aliases in clear-state targets. Tested: packages/vrender-core rushx test --runInBand Tested: packages/vrender-animate rushx test --runInBand Tested: rush compile -t @visactor/vrender-core -t @visactor/vrender-animate Tested: eslint on changed files; git diff --check Tested: rush build -t @visactor/vrender-core -t @visactor/vrender-animate Not-tested: VChart npm consumer smoke; requires alpha.7 package publish.
1 parent f9ebf2b commit e7dd936

6 files changed

Lines changed: 413 additions & 25 deletions

File tree

packages/vrender-animate/__tests__/unit/animation-runtime-attribute.test.ts

Lines changed: 178 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createGroup, createRect } from '@visactor/vrender-core';
1+
import { application, createGroup, createRect, DefaultGraphicService } from '@visactor/vrender-core';
22
import { registerAnimate } from '../../src/register';
33
import { registerCustomAnimate } from '../../src/custom/register';
44
import { AnimateExecutor } from '../../src/executor/animate-executor';
@@ -13,6 +13,9 @@ function ensureAnimationRuntime() {
1313
}
1414
registerAnimate();
1515
registerCustomAnimate();
16+
if (!application.graphicService) {
17+
application.graphicService = new DefaultGraphicService();
18+
}
1619
animationRuntimeRegistered = true;
1720
}
1821

@@ -23,6 +26,9 @@ function createGraphicServiceStub() {
2326
onRemove: jest.fn(),
2427
onAddIncremental: jest.fn(),
2528
onClearIncremental: jest.fn(),
29+
beforeUpdateAABBBounds: jest.fn(),
30+
afterUpdateAABBBounds: jest.fn(),
31+
clearAABBBounds: jest.fn(),
2632
validCheck: jest.fn(() => true)
2733
};
2834
}
@@ -83,6 +89,14 @@ function tick(ticker: ManualTicker, delta: number) {
8389
ticker.tick(delta);
8490
}
8591

92+
function boundsSize(graphic: any) {
93+
const bounds = graphic.AABBBounds;
94+
return {
95+
width: bounds.x2 - bounds.x1,
96+
height: bounds.y2 - bounds.y1
97+
};
98+
}
99+
86100
describe('D3 pre-handoff animation runtime', () => {
87101
beforeAll(() => {
88102
ensureAnimationRuntime();
@@ -249,6 +263,169 @@ describe('D3 pre-handoff animation runtime', () => {
249263
expect(rect.resolvedStatePatch).toBeUndefined();
250264
});
251265

266+
test('clearing hover state keeps y/y1 rect layout from collapsing through undefined height aliases', () => {
267+
const { group, ticker, graphicService } = createStageHarness('state-runtime-clear-vertical-alias');
268+
const rect = createRect({
269+
x: 150,
270+
y: 0,
271+
y1: 320,
272+
width: 140,
273+
x1: undefined,
274+
height: undefined,
275+
lineWidth: 0,
276+
fill: '#1664FF',
277+
stroke: '#1664FF',
278+
fillOpacity: undefined,
279+
visible: true
280+
} as any);
281+
bindGraphicService(rect as any, graphicService);
282+
rect.setFinalAttributes({ ...rect.attribute });
283+
group.appendChild(rect);
284+
285+
rect.states = {
286+
hover: {
287+
lineWidth: 4,
288+
fillOpacity: 0.6
289+
}
290+
} as any;
291+
rect.stateAnimateConfig = {
292+
duration: 100,
293+
easing: 'linear'
294+
} as any;
295+
296+
rect.useStates(['hover'], true);
297+
tick(ticker, 100);
298+
expect(rect.attribute.lineWidth).toBe(4);
299+
expect(rect.attribute.fillOpacity).toBe(0.6);
300+
expect((rect as any).baseAttributes.height).toBeUndefined();
301+
expect((rect as any).baseAttributes.x1).toBeUndefined();
302+
303+
rect.useStates([], true);
304+
tick(ticker, 16);
305+
306+
expect(rect.attribute.height).toBeUndefined();
307+
expect(rect.attribute.x1).toBeUndefined();
308+
expect(rect.attribute.y).toBe(0);
309+
expect(rect.attribute.y1).toBe(320);
310+
expect(boundsSize(rect).height).toBeGreaterThan(100);
311+
312+
tick(ticker, 100);
313+
expect(rect.attribute.height).toBeUndefined();
314+
expect(rect.attribute.x1).toBeUndefined();
315+
expect((rect as any).baseAttributes.height).toBeUndefined();
316+
expect((rect as any).baseAttributes.x1).toBeUndefined();
317+
expect(rect.getFinalAttribute().height).toBeUndefined();
318+
expect(rect.getFinalAttribute().x1).toBeUndefined();
319+
expect(rect.attribute.y).toBe(0);
320+
expect(rect.attribute.y1).toBe(320);
321+
expect(rect.attribute.lineWidth).toBe(0);
322+
expect(boundsSize(rect).height).toBeGreaterThan(100);
323+
});
324+
325+
test('clearing hover state keeps x/x1 rect layout from collapsing through undefined width aliases', () => {
326+
const { group, ticker, graphicService } = createStageHarness('state-runtime-clear-horizontal-alias');
327+
const rect = createRect({
328+
x: 10,
329+
x1: 210,
330+
y: 5,
331+
height: 40,
332+
width: undefined,
333+
y1: undefined,
334+
lineWidth: 0,
335+
fill: '#1664FF',
336+
stroke: '#1664FF',
337+
fillOpacity: undefined,
338+
visible: true
339+
} as any);
340+
bindGraphicService(rect as any, graphicService);
341+
rect.setFinalAttributes({ ...rect.attribute });
342+
group.appendChild(rect);
343+
344+
rect.states = {
345+
hover: {
346+
lineWidth: 4,
347+
fillOpacity: 0.6
348+
}
349+
} as any;
350+
rect.stateAnimateConfig = {
351+
duration: 100,
352+
easing: 'linear'
353+
} as any;
354+
355+
rect.useStates(['hover'], true);
356+
tick(ticker, 100);
357+
expect(rect.attribute.lineWidth).toBe(4);
358+
expect(rect.attribute.fillOpacity).toBe(0.6);
359+
expect((rect as any).baseAttributes.width).toBeUndefined();
360+
expect((rect as any).baseAttributes.y1).toBeUndefined();
361+
362+
rect.useStates([], true);
363+
tick(ticker, 16);
364+
365+
expect(rect.attribute.width).toBeUndefined();
366+
expect(rect.attribute.y1).toBeUndefined();
367+
expect(rect.attribute.x).toBe(10);
368+
expect(rect.attribute.x1).toBe(210);
369+
expect(boundsSize(rect).width).toBeGreaterThan(100);
370+
371+
tick(ticker, 100);
372+
expect(rect.attribute.width).toBeUndefined();
373+
expect(rect.attribute.y1).toBeUndefined();
374+
expect((rect as any).baseAttributes.width).toBeUndefined();
375+
expect((rect as any).baseAttributes.y1).toBeUndefined();
376+
expect(rect.getFinalAttribute().width).toBeUndefined();
377+
expect(rect.getFinalAttribute().y1).toBeUndefined();
378+
expect(rect.attribute.x).toBe(10);
379+
expect(rect.attribute.x1).toBe(210);
380+
expect(rect.attribute.lineWidth).toBe(0);
381+
expect(boundsSize(rect).width).toBeGreaterThan(100);
382+
});
383+
384+
test('clearing hover state animates removed style keys to defaults when base lacks those keys', () => {
385+
const { group, ticker, graphicService } = createStageHarness('state-runtime-clear-missing-style-key');
386+
const rect = createRect({
387+
x: 0,
388+
y: 0,
389+
y1: 100,
390+
width: 20,
391+
lineWidth: 0,
392+
fill: '#1664FF',
393+
visible: true
394+
} as any);
395+
bindGraphicService(rect as any, graphicService);
396+
rect.setFinalAttributes({ ...rect.attribute });
397+
group.appendChild(rect);
398+
399+
rect.states = {
400+
hover: {
401+
lineWidth: 4,
402+
fillOpacity: 0.6
403+
}
404+
} as any;
405+
rect.stateAnimateConfig = {
406+
duration: 100,
407+
easing: 'linear'
408+
} as any;
409+
410+
rect.useStates(['hover'], true);
411+
tick(ticker, 100);
412+
expect(rect.attribute.fillOpacity).toBe(0.6);
413+
expect((rect as any).baseAttributes.fillOpacity).toBeUndefined();
414+
415+
rect.useStates([], true);
416+
tick(ticker, 16);
417+
418+
expect(rect.attribute.fillOpacity).toBeGreaterThan(0.6);
419+
expect(rect.attribute.fillOpacity).toBeLessThan(1);
420+
expect((rect as any).baseAttributes.fillOpacity).toBeUndefined();
421+
expect(rect.getFinalAttribute().fillOpacity).toBeUndefined();
422+
423+
tick(ticker, 100);
424+
expect(rect.attribute.fillOpacity).toBeUndefined();
425+
expect((rect as any).baseAttributes.fillOpacity).toBeUndefined();
426+
expect(rect.getFinalAttribute().fillOpacity).toBeUndefined();
427+
});
428+
252429
test('animate.to restores static truth after completion and keeps baseAttributes untouched', () => {
253430
const { group, ticker, graphicService } = createStageHarness('self-to');
254431
const rect = createAnimatedRect(graphicService);

packages/vrender-animate/src/animate.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,7 @@ export class Animate implements IAnimate {
624624
if (typeof trackerTarget?.restoreStaticAttribute === 'function') {
625625
trackerTarget.restoreStaticAttribute();
626626
}
627+
(this as any).__skipRestoreStaticAttributeOnRemove = true;
627628
return;
628629
}
629630

packages/vrender-core/__tests__/unit/graphic/state-animation.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,42 @@ describe('Graphic state animation integration', () => {
243243
});
244244
});
245245

246+
test('should animate removed clear-state style keys back to defaults when base lacks the key', () => {
247+
const graphic = createGraphic();
248+
delete (graphic as any).baseAttributes.fillOpacity;
249+
delete (graphic.attribute as any).fillOpacity;
250+
graphic.setFinalAttributes?.({ ...(graphic.attribute as any) });
251+
graphic.states = {
252+
hover: {
253+
lineWidth: 4,
254+
fillOpacity: 0.6
255+
}
256+
} as any;
257+
const applyAnimationState = jest.fn();
258+
(graphic as any).applyAnimationState = applyAnimationState;
259+
260+
graphic.useStates(['hover'], true);
261+
graphic.clearStates(true);
262+
263+
expect(applyAnimationState).toHaveBeenLastCalledWith(
264+
['state'],
265+
[
266+
{
267+
name: 'state',
268+
animation: {
269+
type: 'state',
270+
to: expect.objectContaining({
271+
lineWidth: 1,
272+
fillOpacity: graphic.getDefaultAttribute('fillOpacity')
273+
}),
274+
duration: DefaultStateAnimateConfig.duration,
275+
easing: DefaultStateAnimateConfig.easing
276+
}
277+
}
278+
]
279+
);
280+
});
281+
246282
test('should allow explicit animateConfig to override graphic and context defaults in applyStateAttrs', () => {
247283
const graphic = createGraphic();
248284
graphic.stateAnimateConfig = {

packages/vrender-core/__tests__/unit/graphic/state-transition-orchestrator.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,88 @@ describe('StateTransitionOrchestrator', () => {
204204
});
205205
});
206206

207+
test('should not materialize undefined rect geometry aliases to theme defaults during clear transitions', () => {
208+
const orchestrator = new StateTransitionOrchestrator<any>();
209+
210+
const verticalPlan = orchestrator.analyzeTransition(
211+
{},
212+
{
213+
lineWidth: 0,
214+
fillOpacity: undefined,
215+
x: 150,
216+
y: 0,
217+
y1: 320,
218+
width: 140,
219+
x1: undefined,
220+
height: undefined
221+
},
222+
[],
223+
true,
224+
{
225+
isClear: true,
226+
getDefaultAttribute: (key: string) => {
227+
if (key === 'lineWidth' || key === 'height' || key === 'x1') {
228+
return 0;
229+
}
230+
if (key === 'fillOpacity') {
231+
return 1;
232+
}
233+
return undefined;
234+
}
235+
}
236+
);
237+
238+
expect(verticalPlan.animateAttrs).toEqual({
239+
lineWidth: 0,
240+
fillOpacity: 1,
241+
x: 150,
242+
y: 0,
243+
y1: 320,
244+
width: 140
245+
});
246+
expect(verticalPlan.animateAttrs).not.toHaveProperty('height');
247+
expect(verticalPlan.animateAttrs).not.toHaveProperty('x1');
248+
249+
const horizontalPlan = orchestrator.analyzeTransition(
250+
{},
251+
{
252+
lineWidth: 0,
253+
fillOpacity: undefined,
254+
x: 10,
255+
x1: 210,
256+
y: 5,
257+
height: 40,
258+
width: undefined,
259+
y1: undefined
260+
},
261+
[],
262+
true,
263+
{
264+
isClear: true,
265+
getDefaultAttribute: (key: string) => {
266+
if (key === 'lineWidth' || key === 'width' || key === 'y1') {
267+
return 0;
268+
}
269+
if (key === 'fillOpacity') {
270+
return 1;
271+
}
272+
return undefined;
273+
}
274+
}
275+
);
276+
277+
expect(horizontalPlan.animateAttrs).toEqual({
278+
lineWidth: 0,
279+
fillOpacity: 1,
280+
x: 10,
281+
x1: 210,
282+
y: 5,
283+
height: 40
284+
});
285+
expect(horizontalPlan.animateAttrs).not.toHaveProperty('width');
286+
expect(horizontalPlan.animateAttrs).not.toHaveProperty('y1');
287+
});
288+
207289
test('should allow animateConfig to append additional no-animate attrs', () => {
208290
const orchestrator = new StateTransitionOrchestrator<any>();
209291

@@ -238,4 +320,32 @@ describe('StateTransitionOrchestrator', () => {
238320
visible: false
239321
});
240322
});
323+
324+
test('should keep extra animation attrs out of the static final target', () => {
325+
const orchestrator = new StateTransitionOrchestrator<any>();
326+
327+
const plan = orchestrator.analyzeTransition(
328+
{},
329+
{
330+
lineWidth: 0
331+
},
332+
[],
333+
true,
334+
{
335+
isClear: true,
336+
extraAnimateAttrs: {
337+
fillOpacity: 1
338+
}
339+
}
340+
);
341+
342+
expect(plan.targetAttrs).toEqual({
343+
lineWidth: 0
344+
});
345+
expect(plan.animateAttrs).toEqual({
346+
lineWidth: 0,
347+
fillOpacity: 1
348+
});
349+
expect(plan.targetAttrs).not.toHaveProperty('fillOpacity');
350+
});
241351
});

0 commit comments

Comments
 (0)