bito-code-review[bot] commented on PR #41434:
URL: https://github.com/apache/superset/pull/41434#issuecomment-5431431591
<!-- Bito Reply -->
The flagged issue is correct. Relying solely on `offsetWidth` can lead to
false positives because CSS layout changes (like viewport resizing) can reduce
the container's width without the ECharts instance actually resizing its
internal canvas. Adding an assertion that `scrollWidth` equals `offsetWidth`
ensures the chart content has successfully reflowed to fit the new container
size.
To resolve this, update the assertion in
`superset-frontend/playwright/tests/dashboard/dashboard-tabs.spec.ts` as
follows:
```typescript
await expect
.poll(
async () => {
const width = await echartsHost.evaluate((el: HTMLElement) =>
el.offsetWidth);
const scrollWidth = await echartsHost.evaluate((el: HTMLElement)
=> el.scrollWidth);
return { width, scrollWidth };
},
{
timeout: TIMEOUT.API_RESPONSE,
message: 'treemap should resize and not overflow after the hidden
tab is revealed',
},
)
.toMatchObject({ width: expect.toBeLessThan(widthAtWide), scrollWidth:
expect.any(Number) });
// Explicitly verify no overflow
const { width, scrollWidth } = await echartsHost.evaluate((el:
HTMLElement) => ({
width: el.offsetWidth,
scrollWidth: el.scrollWidth,
}));
expect(scrollWidth).toBe(width);
```
There are no other comments on this PR. Would you like me to implement this
fix for you?
**superset-frontend/playwright/tests/dashboard/dashboard-tabs.spec.ts**
```
await expect
.poll(
async () => {
const width = await echartsHost.evaluate((el: HTMLElement) =>
el.offsetWidth);
const scrollWidth = await echartsHost.evaluate((el: HTMLElement)
=> el.scrollWidth);
return { width, scrollWidth };
},
{
timeout: TIMEOUT.API_RESPONSE,
message: 'treemap should resize and not overflow after the hidden
tab is revealed',
},
)
.toMatchObject({ width: expect.toBeLessThan(widthAtWide), scrollWidth:
expect.any(Number) });
// Explicitly verify no overflow
const { width, scrollWidth } = await echartsHost.evaluate((el:
HTMLElement) => ({
width: el.offsetWidth,
scrollWidth: el.scrollWidth,
}));
expect(scrollWidth).toBe(width);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
