Skip to content

Commit e554744

Browse files
authored
refactor: replace _finishTransition with recalculateLayout (#372)
1 parent 6e666de commit e554744

2 files changed

Lines changed: 39 additions & 16 deletions

File tree

packages/react-components/src/MasterDetailLayout.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ function Detail({ children }: DetailProps) {
8787
// _finishTransition) because the WC reads DOM state immediately after.
8888
// React state updates are async, so we manipulate the DOM directly in
8989
// the callback, then sync React state afterwards.
90-
layout._startTransition(transitionType, () => {
90+
layout._startTransition(transitionType, async () => {
9191
if (transitionType === 'replace' && currentDetailsRef.current) {
9292
// For replace, _startTransition moved the current div to the
9393
// detail-outgoing slot for the outgoing animation. Clone it so the
@@ -105,8 +105,12 @@ function Detail({ children }: DetailProps) {
105105
const hasNext = nextDetailsRef.current.childElementCount > 0;
106106
nextDetailsRef.current.setAttribute('slot', hasNext ? 'detail' : 'detail-hidden');
107107
}
108-
// Recompute layout state synchronously with the new DOM
109-
layout._finishTransition();
108+
109+
// Wait for Lit elements to render
110+
await Promise.resolve();
111+
112+
// Recompute layout state with the new DOM
113+
layout.recalculateLayout();
110114
}).then(() => {
111115
// Animation finished — sync React state to match DOM reality
112116
setCurrentChildren(children);

test/MasterDetailLayout.spec.tsx

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,31 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
22
import { render, type RenderResult } from 'vitest-browser-react';
33
import type { ReactNode } from 'react';
44
import sinon from 'sinon';
5+
import { LitElement, html } from 'lit';
56
import { MasterDetailLayout, MasterDetailLayoutElement } from '../packages/react-components/src/MasterDetailLayout.js';
67

8+
declare module 'react' {
9+
namespace JSX {
10+
interface IntrinsicElements {
11+
'test-lit-element': React.DetailedHTMLProps<React.HTMLAttributes<HTMLElement>, HTMLElement>;
12+
}
13+
}
14+
}
15+
16+
class TestLitElement extends LitElement {
17+
override render() {
18+
return html`<div style="width: 200px">Lit Content</div>`;
19+
}
20+
}
21+
22+
customElements.define('test-lit-element', TestLitElement);
23+
724
window.Vaadin ||= {};
825
window.Vaadin.featureFlags ||= {};
926
window.Vaadin.featureFlags.masterDetailLayoutComponent = true;
1027

1128
describe('MasterDetailLayout', () => {
1229
let startTransitionSpy: sinon.SinonSpy;
13-
let finishTransitionSpy: sinon.SinonSpy;
1430
let result: RenderResult;
1531
let layout: MasterDetailLayoutElement;
1632

@@ -32,7 +48,6 @@ describe('MasterDetailLayout', () => {
3248
beforeEach(async () => {
3349
result = await render(<MasterDetailLayout></MasterDetailLayout>);
3450
startTransitionSpy = sinon.spy();
35-
finishTransitionSpy = sinon.spy();
3651

3752
layout = document.querySelector('vaadin-master-detail-layout')!;
3853
expect(layout).to.exist;
@@ -42,10 +57,6 @@ describe('MasterDetailLayout', () => {
4257
callback();
4358
return Promise.resolve();
4459
};
45-
(layout as any)._finishTransition = () => {
46-
finishTransitionSpy();
47-
return Promise.resolve();
48-
};
4960
});
5061

5162
it('should render master and detail content', async () => {
@@ -281,27 +292,35 @@ describe('MasterDetailLayout', () => {
281292
expect(startTransitionSpy.firstCall.args[0]).to.equal('remove');
282293
});
283294

284-
it('should properly start and finish transitions', async () => {
295+
it('should call recalculateLayout after Lit elements have rendered', async () => {
296+
// Use the real _startTransition
297+
delete (layout as any)._startTransition;
298+
299+
layout.style.width = '800px';
300+
285301
// Start without content
286302
await result.rerender(
287303
<MasterDetailLayout>
288304
<MasterDetailLayout.Detail />
289305
</MasterDetailLayout>,
290306
);
291307

292-
// Change detail content to trigger transition
308+
// Add a Lit element as detail content. Its intrinsic width should be measured
309+
// correctly only if recalculateLayout is called after the Lit element renders.
293310
await result.rerender(
294311
<MasterDetailLayout>
295312
<MasterDetailLayout.Detail>
296-
<div>New Content</div>
313+
<test-lit-element />
297314
</MasterDetailLayout.Detail>
298315
</MasterDetailLayout>,
299316
);
300317

301-
await assertDetailsVisible('New Content');
302-
expect(startTransitionSpy.calledOnce).to.be.true;
303-
expect(finishTransitionSpy.calledOnce).to.be.true;
304-
expect(startTransitionSpy.calledBefore(finishTransitionSpy)).to.be.true;
318+
const litElement = layout.querySelector('test-lit-element') as TestLitElement;
319+
await vi.waitFor(() => {
320+
expect(litElement.shadowRoot!.querySelector('div')).to.exist;
321+
});
322+
323+
expect(getComputedStyle(layout).getPropertyValue('--_detail-cached-size')).to.equal('201px'); // 1px border
305324
});
306325

307326
describe('Child validation', () => {

0 commit comments

Comments
 (0)