Skip to content

Commit

Permalink
Code review comments (deephaven#1890)
Browse files Browse the repository at this point in the history
  • Loading branch information
bmingles committed Apr 30, 2024
1 parent c02cd50 commit 89aa1e0
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 0 deletions.
12 changes: 12 additions & 0 deletions packages/components/src/spectrum/listView/ListViewNormalized.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ export interface ListViewNormalizedProps
showItemIcons: boolean;
}

/**
* ListView supporting normalized item data. This component mostly exists to
* decouple some of the logic needed to support table data. Specifically it
* handles item rendering configurations as well as converting selection keys
* to / from strings. This makes it easier to test logic in isolation without
* a dependency on JS apis (e.g. in the Styleguide).
*
* Note that This component will usually not be used directly. Instead, it is
* recommended to use
* - `@deephaven/components`'s `ListView` for non-table data sources
* - `@deephaven/jsapi-components`'s `ListView` for table data sources
*/
export function ListViewNormalized({
normalizedItems,
tooltip = true,
Expand Down
11 changes: 11 additions & 0 deletions packages/components/src/spectrum/listView/ListViewWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ export interface ListViewWrapperProps<T> extends SpectrumListViewProps<T> {
onScroll?: (event: Event) => void;
}

/**
* Helper component to wrap a ListView with the appropriate styling + scroll
* handling. This is used by both the `@deephaven/components` `ListView` and
* the `@deephaven/jsapi-components` `ListView` (via `ListViewNormalized`) to
* ensure consistency.
*
* Note that This component will usually not be used directly. Instead, it is
* recommended to use
* - `@deephaven/components`'s `ListView` for non-table data sources
* - `@deephaven/jsapi-components`'s `ListView` for table data sources
*/
export function ListViewWrapper<T>(
props: ListViewWrapperProps<T>
): JSX.Element {
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/spectrum/utils/itemWrapperUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ export function wrapPrimitiveWithText(
if (['string', 'boolean', 'number'].includes(typeof content)) {
return (
<Text slot={slot}>
{/* Non-breaking space is needed to avoid the Text element's height
collapsing when content is empty */}
{content === '' ? NON_BREAKING_SPACE : String(content)}
</Text>
);
Expand Down

0 comments on commit 89aa1e0

Please sign in to comment.