Skip to content

Commit

Permalink
Merge pull request #193 from hypothesis/table-updates
Browse files Browse the repository at this point in the history
Add `emptyItemsMessage`, reduce CSS specificity for `Table` (`table` pattern)
  • Loading branch information
lyzadanger authored Sep 15, 2021
2 parents 32e2999 + a50b437 commit bb837d2
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 48 deletions.
13 changes: 12 additions & 1 deletion src/components/Table.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { Scrollbox } from './containers';
* @prop {string} [classes] - Extra CSS classes to apply to the <table>
* @prop {string} [containerClasses] - Extra CSS classes to apply to the outermost
* element, which is a <Scrollbox> div
* @prop {import("preact").ComponentChildren} [emptyItemsMessage] - Optional message to display if
* there are no `items`. Will only display when the Table is not loading.
* @prop {TableHeader[]} tableHeaders - The columns to display in this table
* @prop {boolean} [isLoading] - Show an indicator that data for the table is
* currently being fetched
Expand Down Expand Up @@ -73,6 +75,7 @@ export function Table({
accessibleLabel,
classes,
containerClasses,
emptyItemsMessage,
isLoading = false,
items,
onSelectItem,
Expand Down Expand Up @@ -176,7 +179,7 @@ export function Table({
{tableHeaders.map(({ classes, label }, index) => (
<th
key={`${label}-${index}`}
className={classnames(classes)}
className={classnames('Hyp-Table__header', classes)}
scope="col"
>
{label}
Expand Down Expand Up @@ -209,6 +212,14 @@ export function Table({
<Spinner size="large" />
</div>
)}
{!isLoading && items.length === 0 && emptyItemsMessage && (
<div
className="Hyp-Table-Scrollbox__message"
data-testid="empty-items-message"
>
{emptyItemsMessage}
</div>
)}
</Scrollbox>
);
}
22 changes: 22 additions & 0 deletions src/components/test/Table-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,19 @@ describe('Table', () => {
const columns = wrapper.find('thead th').map(col => col.text());
assert.deepEqual(columns, ['Name', 'Size']);
});

it('does not show an empty items message', () => {
const wrapper = createComponent({
isLoading: true,
tableHeaders: [{ label: 'Name' }, { label: 'Size' }],
items: [],
emptyItemsMessage: 'There is nothing here',
});

assert.isFalse(
wrapper.find('[data-testid="empty-items-message"]').exists()
);
});
});

it('renders column headings', () => {
Expand All @@ -91,6 +104,15 @@ describe('Table', () => {
assert.isTrue(wrapper.contains(<td>Three</td>));
});

it('shows provided empty message if there are no items', () => {
const wrapper = createComponent({
items: [],
emptyItemsMessage: 'There is nothing here',
});

assert.isTrue(wrapper.find('[data-testid="empty-items-message"]').exists());
});

['click', 'mousedown'].forEach(event => {
it(`selects item on ${event}`, () => {
const onSelectItem = sinon.stub();
Expand Down
62 changes: 60 additions & 2 deletions src/pattern-library/components/patterns/TableComponents.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ const renderCallback = file => (
</>
);

const customizedRenderCallback = file => (
<>
<td className="hyp-u-color--grey-6">{file.displayName}</td>
<td>{file.updated}</td>
</>
);

const { tableHeaders, items } = sampleTableContent();

function TableExample() {
Expand Down Expand Up @@ -64,7 +71,13 @@ function ScrollboxTableExample() {
scroll if it overflows. Apply height/width constraints to an appropriate
parent elements to enable this. Height will not change when loading.
</p>
<p>In this example, the last item in the table is pre-selected.</p>
<p>
In this example, the last item in the table is pre-selected. Also in
this example: an additional style is added to the first <code>td</code>{' '}
in each row to make its foreground color different (NB: the example here
would not meet ARIA contrast requirements). This demonstrates style
extension/override.
</p>
<Library.Demo withSource>
<div className="hyp-u-padding--5">
<LabeledButton onClick={() => setIsLoading(!isLoading)}>
Expand All @@ -82,7 +95,7 @@ function ScrollboxTableExample() {
selectedItem={selectedFile}
onSelectItem={file => setSelectedFile(file)}
onUseItem={file => alert(`Selected ${file.displayName}`)}
renderItem={file => renderCallback(file)}
renderItem={file => customizedRenderCallback(file)}
tableHeaders={tableHeaders}
/>
</div>
Expand All @@ -91,12 +104,57 @@ function ScrollboxTableExample() {
);
}

function EmptyTableExample() {
const [isLoading, setIsLoading] = useState(false);
const items = [];
const [selectedFile, setSelectedFile] = useState(
/** @type {null|object} */ (items[items.length - 1])
);

const emptyItemsMessage = (
<p>
There are no files available to show.{' '}
<a href="https://www.example.com">Learn more.</a>
</p>
);

return (
<Library.Example title="Constrained Table" variant="wide">
<p>
This Table has no items (it is empty). When not in loading state, the
provided <code>emptyItemsMessage</code> will render centered in the
table.
</p>
<Library.Demo withSource>
<div className="hyp-u-padding--5">
<LabeledButton onClick={() => setIsLoading(!isLoading)}>
Toggle Loading
</LabeledButton>
</div>

<Table
accessibleLabel="File list"
emptyItemsMessage={emptyItemsMessage}
isLoading={isLoading}
items={items}
selectedItem={selectedFile}
onSelectItem={file => setSelectedFile(file)}
onUseItem={file => alert(`Selected ${file.displayName}`)}
renderItem={file => renderCallback(file)}
tableHeaders={tableHeaders}
/>
</Library.Demo>
</Library.Example>
);
}

export default function TableComponents() {
return (
<Library.Page title="Table">
<Library.Pattern title="Table">
<TableExample />
<ScrollboxTableExample />
<EmptyTableExample />
</Library.Pattern>
</Library.Page>
);
Expand Down
28 changes: 22 additions & 6 deletions src/pattern-library/components/patterns/TablePatterns.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ export default function TablePatterns() {
<table className="hyp-table">
<thead>
<tr>
<th scope="col">Column A</th>
<th scope="col">Column B</th>
<th scope="col" className="hyp-table__header">
Column A
</th>
<th scope="col" className="hyp-table__header">
Column B
</th>
</tr>
</thead>
<SampleTableBody />
Expand All @@ -42,10 +46,18 @@ export default function TablePatterns() {
<table className="hyp-table">
<thead>
<tr>
<th scope="col" style="width:30%">
<th
scope="col"
className="hyp-table__header"
style="width:30%"
>
Column A
</th>
<th scope="col" style="width:70%">
<th
scope="col"
className="hyp-table__header"
style="width:70%"
>
Column B
</th>
</tr>
Expand All @@ -68,8 +80,12 @@ export default function TablePatterns() {
<table className="hyp-table">
<thead>
<tr>
<th scope="col">Column A</th>
<th scope="col">Column B</th>
<th scope="col" className="hyp-table__header">
Column A
</th>
<th scope="col" className="hyp-table__header">
Column B
</th>
</tr>
</thead>
<SampleTableBody />
Expand Down
5 changes: 5 additions & 0 deletions styles/components/Table.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ $-row-height: var.$touch-target-size;
// Adjust vertical position to account for header, which takes up one "row"
margin-top: math.div($-row-height, 2);
}

&__message {
@include layout.absolute-centered;
top: $-row-height * 2;
}
}

.Hyp-Table {
Expand Down
76 changes: 37 additions & 39 deletions styles/mixins/patterns/_tables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,56 +38,54 @@ $-min-row-height: var.$touch-target-size;
th,
td {
@include layout.padding;
// Prevent extra vertical height with <th> elements
// FIXME: Review after typography patterns introduced
line-height: 1;
}

td {
@include atoms.border(top);
}

thead {
// Prevent extra vertical height with <th> elements
// FIXME: Review after typography patterns introduced
line-height: 1;

th {
@include containers.sticky-header;
border-bottom: 1px solid $-color-header-border;
background-color: $-color-header-background;
// Ensure the header is displayed above content in the table when it is
// scrolled, including any content which establishes a new stacking context.
z-index: 1;
text-align: left;
}
&__header {
@include containers.sticky-header;
border-bottom: 1px solid $-color-header-border;
background-color: $-color-header-background;
// Ensure the header is displayed above content in the table when it is
// scrolled, including any content which establishes a new stacking context.
z-index: 1;
text-align: left;
}

tbody {
// Make table content look interact-able
cursor: pointer;
}

tr {
height: $-min-row-height;
@include layout.padding;
}

// No border on top of first row's <td> elements, to eliminate a
// double border with the <th>s
tr:first-child td {
border-top: none;
}

tr.is-selected td {
background-color: $-color-background--inverted;
color: $-color-text--inverted;
}

tr:hover:not(.is-selected) {
background-color: $-color-background-hover;
}

// No border on top of first row's <td> elements, to eliminate a
// double border with the <th>s
& tr:first-child td {
border-top: none;
}

& tr:nth-child(odd) {
// Need a low-opacity black versus a named greyscale color because
// this background needs to be very low opacity so as not to obscure
// scroll-hinting shadows
background-color: rgba(0, 0, 0, 0.025);
}

& tr {
height: $-min-row-height;

&.is-selected {
background-color: $-color-background--inverted;
color: $-color-text--inverted;
}
}

& tr:hover:not(.is-selected) {
background-color: $-color-background-hover;
}
tr:nth-child(odd):not(.is-selected) {
// Need a low-opacity black versus a named greyscale color because
// this background needs to be very low opacity so as not to obscure
// scroll-hinting shadows
background-color: rgba(0, 0, 0, 0.025);
}
}

0 comments on commit bb837d2

Please sign in to comment.