Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOP-5110: Render sub rows for ListTable #1342

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 86 additions & 38 deletions src/components/ListTable.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useMemo, useRef } from 'react';
import React, { useMemo, useRef, useState } from 'react';
import PropTypes from 'prop-types';
import {
Cell,
Expand Down Expand Up @@ -32,17 +32,24 @@ const styleTable = ({ customAlign, customWidth }) => css`
${customAlign && `text-align: ${align(customAlign)}`};
${customWidth && `width: ${customWidth}`};
margin: ${theme.size.medium} 0;

tbody[data-expanded='true'] {
// Avoid flash of light mode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

.dark-theme & {
background-color: ${palette.gray.dark4};
}
}
`;

const theadStyle = css`
// Allows its box shadow to appear above stub cell's background color
position: relative;
color: var(--font-color-primary);
background-color: ${palette.white};
// Allow nested tables to inherit background of the current row
background-color: inherit;
box-shadow: 0 ${theme.size.tiny} ${palette.gray.light2};

.dark-theme & {
background-color: ${palette.black};
box-shadow: 0 ${theme.size.tiny} ${palette.gray.dark2};
}
`;
Expand All @@ -57,13 +64,14 @@ const baseCellStyle = css`
* {
// Wrap in selector to ensure it cascades down to every element
font-size: ${theme.fontSize.small} !important;
line-height: inherit;
line-height: 20px !important;
}

// Ensure each cell is no higher than the highest content in row
& > div {
height: unset;
min-height: unset;
max-height: unset;
}
`;

Expand All @@ -72,29 +80,16 @@ const bodyCellStyle = css`
word-break: break-word;
align-content: flex-start;

& > div {
min-height: unset;
max-height: unset;
flex-direction: column;
align-items: flex-start;
}

*,
p,
a {
line-height: 20px;
}

// Target any nested components (paragraphs, admonitions, tables) and any paragraphs within those nested components
& > div > *,
& > div > div p {
margin: 0 0 12px;
& > *,
& > div p {
margin: 0 0 12px !important;
}

// Prevent extra margin below last element (such as when multiple paragraphs are present)
& > div > div *:last-child,
& > div > *:last-child {
margin-bottom: 0;
& > div *:last-child,
& > *:last-child {
margin-bottom: 0 !important;
}
`;

Expand All @@ -116,6 +111,14 @@ const stubCellStyle = css`
}
`;

const subRowStyle = css`
// For some reason, collapsed subrows were causing the row to still take up space,
// so this is our workaround for now
&[aria-hidden='true'] {
display: none;
}
`;

const zebraStripingStyle = css`
&:nth-of-type(even) {
background-color: ${palette.gray.light3};
Expand Down Expand Up @@ -209,24 +212,43 @@ const generateColumns = (headerRow, bodyRows) => {
});
};

const generateRowsData = (bodyRowNodes, columns) => {
const rowNodes = bodyRowNodes.map((node) => node?.children[0]?.children ?? []);
const rows = rowNodes.map((rowNode) => {
return rowNode.reduce((res, columnNode, colIndex) => {
const column = columns[colIndex];
const generateRowsData = (rowNodes, columns, isNested = false) => {
const rows = rowNodes.map((row) => {
const res = {};
const cells = [];
const nestedRows = [];

const potentialCells = isNested ? row.children : row;
for (const item of potentialCells) {
if (item.type === 'directive' && item.name === 'row') {
nestedRows.push(item);
} else {
cells.push(item);
}
}

cells.forEach((cell, colIdx) => {
const column = columns[colIdx];
if (!column) {
console.warn(`Row has too many items (index ${colIndex}) for table with ${columns.length} columns`);
console.warn(`Row has too many items (index ${colIdx}) for table with ${columns.length} columns`);
return res;
}
res[column?.accessorKey ?? colIndex] = (

const skipPTag = hasOneChild(cell.children);
res[column.accessorKey ?? colIdx] = (
<>
{columnNode.children.map((cellNode, index) => (
<ComponentFactory key={index} nodeData={cellNode} />
{cell.children.map((contentNode, index) => (
<ComponentFactory key={index} nodeData={contentNode} skipPTag={skipPTag} />
))}
</>
);
return res;
}, {});
});

if (nestedRows.length > 0) {
res['subRows'] = generateRowsData(nestedRows, columns, true);
}

return res;
});

return rows;
Expand Down Expand Up @@ -259,14 +281,26 @@ const ListTable = ({ nodeData: { children, options }, ...rest }) => {

const tableRef = useRef();
const columns = useMemo(() => generateColumns(headerRows[0], bodyRows), [bodyRows, headerRows]);
const data = useMemo(() => generateRowsData(bodyRows, columns), [bodyRows, columns]);
// Destructure bodyRows' list element structure
const data = useMemo(
() =>
generateRowsData(
bodyRows.map((node) => node?.children[0]?.children ?? []),
columns
),
[bodyRows, columns]
);
const [expanded, setExpanded] = useState(true);
const table = useLeafyGreenTable({
containerRef: tableRef,
columns: columns,
data: data,
state: {
expanded,
},
onExpandedChange: setExpanded,
});
const { rows } = table.getRowModel();

const columnCount = columns.length;

let widths = null;
Expand All @@ -285,6 +319,7 @@ const ListTable = ({ nodeData: { children, options }, ...rest }) => {
<div className="header-buffer" key={id} id={id} />
))}
<Table
table={table}
className={cx(
styleTable({
customAlign: options?.align,
Expand Down Expand Up @@ -322,11 +357,24 @@ const ListTable = ({ nodeData: { children, options }, ...rest }) => {
const isStub = colIndex <= stubColumnCount - 1;
const role = isStub ? 'rowheader' : null;
return (
<Cell key={cell.id} className={cx(baseCellStyle, bodyCellStyle, isStub && stubCellStyle)} role={role}>
{cell.renderValue()}
<Cell key={cell.id} className={cx(baseCellStyle, isStub && stubCellStyle)} role={role}>
<div className={cx(bodyCellStyle)}>{cell.renderValue()}</div>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to wrap cell content within a new div to ensure cell content was contained and styled separately. Whenever a row has sub rows, the "expand row" button is added as the first child of the Cell and the previous styling would cause layout issues

</Cell>
);
})}

{row.subRows &&
row.subRows.map((subRow) => (
<Row key={subRow.id} row={subRow} className={cx(subRowStyle)}>
{subRow.getVisibleCells().map((cell) => {
return (
<Cell key={cell.id} className={cx(baseCellStyle)}>
<div className={cx(bodyCellStyle)}>{cell.renderValue()}</div>
</Cell>
);
})}
</Row>
))}
</Row>
))}
</TableBody>
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/ListTable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import ListTable from '../../src/components/ListTable';

import mockData from './data/ListTable.test.json';
import mockDataFixedWidths from './data/ListTableFixedWidths.test.json';
import mockDataNestedRows from './data/ListTableNestedRows.test.json';

expect.extend(matchers);

Expand Down Expand Up @@ -63,4 +64,12 @@ describe('when rendering a list table with fixed widths', () => {
const wrapper = mountListTable(data);
expect(wrapper.queryAllByRole('rowheader')).toHaveLength(0);
});

describe('when rendering a list table with nested rows', () => {
const data = mockDataNestedRows;
it('renders correctly', () => {
const wrapper = mountListTable(data);
expect(wrapper.asFragment()).toMatchSnapshot();
});
});
});
Loading