Skip to content

Commit

Permalink
Fixed styleguide crashing (deephaven#1890)
Browse files Browse the repository at this point in the history
  • Loading branch information
bmingles committed Apr 30, 2024
1 parent 89aa1e0 commit 06617ed
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 7 deletions.
36 changes: 29 additions & 7 deletions packages/components/src/spectrum/listView/ListViewWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
} from '@adobe/react-spectrum';
import {
extractSpectrumHTMLElement,
useContentRect,
useOnScrollRef,
} from '@deephaven/react-hooks';
import { EMPTY_FUNCTION } from '@deephaven/utils';
Expand Down Expand Up @@ -41,25 +42,46 @@ export function ListViewWrapper<T>(

const scrollRef = useOnScrollRef(onScroll, extractSpectrumHTMLElement);

// Spectrum ListView crashes when it has zero height. Track the contentRect
// of the parent container and only render the ListView when it has a non-zero
// height. See https://github.com/adobe/react-spectrum/issues/6213
const { ref: contentRectRef, contentRect } = useContentRect(
extractSpectrumHTMLElement
);

return (
<Flex
direction="column"
ref={contentRectRef}
// eslint-disable-next-line react/jsx-props-no-spreading
{...styleProps}
// Set min-height to 1px so that `ListView` is rendered whenever container
// is visible. This prevents the height from shrinking to zero as a result
// of a parent grid or flex container calculating content sizes. The
// container height can still be zero when it is not being displayed such
// as when one of its parents have `display: none`.
minHeight={styleProps.minHeight ?? 1}
UNSAFE_className={cl(
'dh-list-view-wrapper',
`dh-list-view-wrapper-density-${listViewProps.density ?? 'regular'}`,
`dh-list-view-wrapper-scale-${scale}`,
styleProps.UNSAFE_className
)}
>
<SpectrumListView
ref={scrollRef}
// eslint-disable-next-line react/jsx-props-no-spreading
{...ariaLabelProps}
// eslint-disable-next-line react/jsx-props-no-spreading
{...listViewProps}
/>
{/**
* Only render ListView if parent is visible. Some time in the future we
* should consider using `checkVisibility()` once it has better browser
* support.
*/}
{contentRect.height === 0 ? null : (
<SpectrumListView
ref={scrollRef}
// eslint-disable-next-line react/jsx-props-no-spreading
{...ariaLabelProps}
// eslint-disable-next-line react/jsx-props-no-spreading
{...listViewProps}
/>
)}
</Flex>
);
}
Expand Down
7 changes: 7 additions & 0 deletions tests/styleguide.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ test('UI regression test - Styleguide button section count', async ({
// Iterate over all sample sections and take a screenshot of each one.
sampleSectionIds.forEach(id => {
test(`UI regression test - Styleguide section - ${id}`, async ({ page }) => {
// Fail quickly if console errors are detected
page.on('console', msg => {
if (msg.type() === 'error') {
throw new Error(msg.text());
}
});

// Isolate the section
await page.goto(`/ide/styleguide?isolateSection=true#${id}`);

Expand Down
Binary file modified tests/styleguide.spec.ts-snapshots/pickers-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/styleguide.spec.ts-snapshots/pickers-firefox-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/styleguide.spec.ts-snapshots/pickers-webkit-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 06617ed

Please sign in to comment.