Skip to content

Commit

Permalink
[IndexTable.Row] Fix tone interaction states; remove subdued and …
Browse files Browse the repository at this point in the history 
…`status` (#10705)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

This PR removes the deprecated `status` and `subdued` props. It also
fixes the missing and buggy interaction states when `IndexTable.Row` has
a `tone`.

| Before | After |
|--------|--------|
|<video
src="https://github.com/Shopify/polaris/assets/18447883/2f2e2f76-adbc-4bed-a18b-c09a23b7542e"/>|<video
src="https://github.com/Shopify/polaris/assets/18447883/4b097d18-4c0c-4fec-b561-9fc9ba2c55b9"/>|

### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [x] Updated the component's `README.md` with documentation changes
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Jess Telford <jess.telford@shopify.com>
  • Loading branch information
@chloerice @jesstelford
chloerice and jesstelford committed Oct 3, 2023
1 parent deba43d commit c7c2312
Show file tree
Hide file tree
Showing 8 changed files with 304 additions and 223 deletions.
5 changes: 5 additions & 0 deletions .changeset/dirty-pugs-retire.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': major
---

- Removed the `subdued` and `status` props from `IndexTable.Row`. Use `tone` instead.
313 changes: 146 additions & 167 deletions polaris-react/src/components/IndexTable/IndexTable.scss
View file

Large diffs are not rendered by default.

99 changes: 86 additions & 13 deletions polaris-react/src/components/IndexTable/IndexTable.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ export function WithSubduedRows() {
key={id}
selected={selectedResources.includes(id)}
position={index}
subdued={index === 1 || index === 2}
tone={index === 1 || index === 2 ? 'subdued' : undefined}
>
<IndexTable.Cell>
<Text fontWeight="bold" as="span">
Expand Down Expand Up @@ -1631,7 +1631,7 @@ export function WithRowStatus() {
key={id}
selected={selectedResources.includes(id)}
position={index}
status={status as IndexTableRowProps['status']}
tone={status as IndexTableRowProps['tone']}
>
<IndexTable.Cell>
<Text fontWeight="bold" as="span">
Expand Down Expand Up @@ -1667,7 +1667,7 @@ export function WithRowStatus() {
onSelectionChange={handleSelectionChange}
headings={[
{title: 'Name'},
{title: 'Status'},
{title: 'Tone'},
{title: 'Location'},
{
alignment: 'end',
Expand Down Expand Up @@ -1696,7 +1696,6 @@ export function WithStickyLastColumn() {
location: 'Decatur, USA',
orders: 20,
amountSpent: '$2,400',
status: 'Created',
channel: 'Point of Sale',
paymentStatus: 'Refunded',
fulfillmentStatus: 'Fulfilled',
Expand All @@ -1708,11 +1707,51 @@ export function WithStickyLastColumn() {
location: 'Los Angeles, USA',
orders: 30,
amountSpent: '$140',
status: 'Created',
channel: 'Online Store',
paymentStatus: 'Paid',
fulfillmentStatus: 'Unfulfilled',
},
{
id: '2562',
url: '#',
name: 'Helen Troy',
location: 'Los Angeles, USA',
orders: 30,
amountSpent: '$975',
lastOrderDate: 'May 31, 2023',
status: 'success',
},
{
id: '4102',
url: '#',
name: 'Colm Dillane',
location: 'New York, USA',
orders: 27,
amountSpent: '$2885',
lastOrderDate: 'May 31, 2023',
status: 'critical',
},
{
id: '2564',
url: '#',
name: 'Al Chemist',
location: 'New York, USA',
orders: 19,
amountSpent: '$1,209',
lastOrderDate: 'April 4, 2023',
disabled: true,
status: 'warning',
},
{
id: '2563',
url: '#',
name: 'Larry June',
location: 'San Francisco, USA',
orders: 22,
amountSpent: '$1,400',
lastOrderDate: 'March 19, 2023',
status: 'subdued',
},
];
const resourceName = {
singular: 'customer',
Expand Down Expand Up @@ -1742,6 +1781,7 @@ export function WithStickyLastColumn() {
key={id}
selected={selectedResources.includes(id)}
position={index}
tone={status as IndexTableRowProps['tone']}
>
<IndexTable.Cell>
<Text fontWeight="bold" as="span">
Expand Down Expand Up @@ -1789,7 +1829,7 @@ export function WithStickyLastColumn() {
id: 'amount-spent',
title: 'Amount spent',
},
{title: 'Status'},
{title: 'Tone'},
{title: 'Channel'},
{title: 'Payment status'},
{title: 'Fulfillment status'},
Expand Down Expand Up @@ -3236,7 +3276,7 @@ export function WithZebraStripingAndRowStatus() {
key={id}
selected={selectedResources.includes(id)}
position={index}
status={status as IndexTableRowProps['status']}
tone={status as IndexTableRowProps['tone']}
>
<IndexTable.Cell>
<Text fontWeight="bold" as="span">
Expand Down Expand Up @@ -3297,7 +3337,6 @@ export function WithZebraStripingAndStickyLastColumn() {
location: 'Decatur, USA',
orders: 20,
amountSpent: '$2,400',
status: 'Created',
channel: 'Point of Sale',
paymentStatus: 'Refunded',
fulfillmentStatus: 'Fulfilled',
Expand All @@ -3309,12 +3348,49 @@ export function WithZebraStripingAndStickyLastColumn() {
location: 'Los Angeles, USA',
orders: 30,
amountSpent: '$140',
status: 'Created',
channel: 'Online Store',
paymentStatus: 'Paid',
fulfillmentStatus: 'Unfulfilled',
},
{
id: '2562',
url: '#',
name: 'Helen Troy',
location: 'Los Angeles, USA',
orders: 30,
amountSpent: '$975',
lastOrderDate: 'May 31, 2023',
},
{
id: '4102',
url: '#',
name: 'Colm Dillane',
location: 'New York, USA',
orders: 27,
amountSpent: '$2885',
lastOrderDate: 'May 31, 2023',
},
{
id: '2564',
url: '#',
name: 'Al Chemist',
location: 'New York, USA',
orders: 19,
amountSpent: '$1,209',
lastOrderDate: 'April 4, 2023',
disabled: true,
},
{
id: '2563',
url: '#',
name: 'Larry June',
location: 'San Francisco, USA',
orders: 22,
amountSpent: '$1,400',
lastOrderDate: 'March 19, 2023',
},
];

const resourceName = {
singular: 'customer',
plural: 'customers',
Expand All @@ -3331,7 +3407,6 @@ export function WithZebraStripingAndStickyLastColumn() {
location,
orders,
amountSpent,
status,
channel,
paymentStatus,
fulfillmentStatus,
Expand Down Expand Up @@ -3360,7 +3435,6 @@ export function WithZebraStripingAndStickyLastColumn() {
{amountSpent}
</Text>
</IndexTable.Cell>
<IndexTable.Cell>{status}</IndexTable.Cell>
<IndexTable.Cell>{channel}</IndexTable.Cell>
<IndexTable.Cell>{paymentStatus}</IndexTable.Cell>
<IndexTable.Cell>{fulfillmentStatus}</IndexTable.Cell>
Expand Down Expand Up @@ -3390,7 +3464,6 @@ export function WithZebraStripingAndStickyLastColumn() {
id: 'amount-spent',
title: 'Amount spent',
},
{title: 'Status'},
{title: 'Channel'},
{title: 'Payment status'},
{title: 'Fulfillment status'},
Expand Down Expand Up @@ -3845,7 +3918,7 @@ export function WithSubHeaders() {
}

const selectableRows = rows.filter(({disabled}) => !disabled);
const rowRange: IndexTableRowProps['subHeaderRange'] = [
const rowRange: IndexTableRowProps['selectionRange'] = [
selectableRows.findIndex((row) => row.id === customers[0].id),
selectableRows.findIndex(
(row) => row.id === customers[customers.length - 1].id,
Expand Down
21 changes: 3 additions & 18 deletions polaris-react/src/components/IndexTable/components/Row/Row.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {Range} from '../../../../utilities/index-provider/types';
import styles from '../../IndexTable.scss';

type RowType = 'data' | 'subheader';
type RowStatus = 'success' | 'subdued' | 'critical' | 'warning';
type RowStatus = 'subdued' | 'success' | 'warning' | 'critical';
type TableRowElementType = HTMLTableRowElement & HTMLLIElement;

export interface RowProps {
Expand All @@ -25,17 +25,7 @@ export interface RowProps {
selected?: boolean | 'indeterminate';
/** The zero-indexed position of the row. Used for Shift key multi-selection */
position: number;
/**
* @deprecated The subdued prop has been deprecated. Use the `tone` prop instead.
* Whether the row should be subdued */
subdued?: boolean;
/**
* @deprecated The status prop has been deprecated. Use the `tone` prop instead.
* Whether the row should have a status */
status?: RowStatus;
/**
* Whether the row should visually indicate its status with color */
/** Whether the row should visually indicate its status with a background color */
tone?: RowStatus;
/** Whether the row should be disabled */
disabled?: boolean;
Expand All @@ -60,8 +50,6 @@ export const Row = memo(function Row({
selected,
id,
position,
subdued,
status,
tone,
disabled,
selectionRange,
Expand Down Expand Up @@ -127,12 +115,9 @@ export const Row = memo(function Row({
rowType === 'subheader' && styles['TableRow-subheader'],
selectable && condensed && styles.condensedRow,
selected && styles['TableRow-selected'],
subdued && styles['TableRow-subdued'],
hovered && !condensed && styles['TableRow-hovered'],
disabled && styles['TableRow-disabled'],
status && styles[variationName('status', status)],
tone && styles[variationName('status', tone)],

tone && styles[variationName('tone', tone)],
!selectable &&
!primaryLinkElement.current &&
styles['TableRow-unclickable'],
Expand Down
40 changes: 32 additions & 8 deletions polaris-react/src/components/IndexTable/components/Row/tests/Row.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,37 +430,61 @@ describe('<Row />', () => {
);
});

it('has an undefined status by default', () => {
it('has an undefined tone by default', () => {
const row = mountWithTable(
<Row {...defaultProps}>
<td />
</Row>,
);

expect(row).toHaveReactProps({status: undefined});
expect(row).toHaveReactProps({tone: undefined});
});

it('applies success status styles when status prop is set to "success"', () => {
it('applies subdued tone styles when tone prop is set to "subdued"', () => {
const row = mountWithTable(
<Row {...defaultProps} status="success">
<Row {...defaultProps} tone="subdued">
<td />
</Row>,
);

expect(row).toContainReactComponent('tr', {
className: 'TableRow statusSuccess',
className: 'TableRow toneSubdued',
});
});

it('applies subdued status styles when status prop is set to "subdued"', () => {
it('applies success tone styles when tone prop is set to "success"', () => {
const row = mountWithTable(
<Row {...defaultProps} status="subdued">
<Row {...defaultProps} tone="success">
<td />
</Row>,
);

expect(row).toContainReactComponent('tr', {
className: 'TableRow statusSubdued',
className: 'TableRow toneSuccess',
});
});

it('applies warning tone styles when tone prop is set to "warning"', () => {
const row = mountWithTable(
<Row {...defaultProps} tone="warning">
<td />
</Row>,
);

expect(row).toContainReactComponent('tr', {
className: 'TableRow toneWarning',
});
});

it('applies critical tone styles when tone prop is set to "critical"', () => {
const row = mountWithTable(
<Row {...defaultProps} tone="critical">
<td />
</Row>,
);

expect(row).toContainReactComponent('tr', {
className: 'TableRow toneCritical',
});
});

Expand Down
32 changes: 15 additions & 17 deletions polaris.shopify.com/content/components/tables/index-table.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,21 +195,19 @@ An `IndexTable.Row` is used to render a row representing an item within an `Inde

### IndexTable.Row properties

| Prop | Type | Description |
| --------------------- | ------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --- |
| children | ReactNode | Table header or data cells |
| id | string | A unique identifier for the row |
| selected? | boolean &#124; "indeterminate" | A boolean property indicating whether the row or it's related rows are selected |
| position | number | The zero-indexed position of the row. Used for Shift key multi-selection as well as selection of a range of rows when a `selectionRange` is set. |
| subdued? (deprecated) | boolean | Whether the row should be subdued. Deprecated in favor of `tone`. |
| status? (deprecated) | "subdued" &#124; "success" &#124; "warning" &#124; "critical" | Whether the row should have a status. Deprecated in favor of `tone`. |
| tone? | "subdued" &#124; "success" &#124; "warning" &#124; "critical" | Whether the row should visually indicate its status with color | |
| disabled? | boolean | Whether the row should be disabled |
| selectionRange? | [number, number] | A tuple array with the first and last index of the range of other rows that the row describes. All non-disabled rows in the range are selected when the row with a selection range set is selected. |
| rowType? | "data" &#124; "subheader" | Indicates the relationship or role of the row's contents. A `rowType` of "subheader" looks and behaves the same as the table header. Defaults to "data". |
| accessibilityLabel? | string | Label set on the row's checkbox. Defaults to "Select \{resourceName\}" |
| onClick? | () => void | Callback fired when the row is clicked. Overrides the default click behaviour. |
| onNavigation? | (id: string) => void | Callback fired when the row is clicked and contains an anchor element with the `data-primary-link` property set |
| Prop | Type | Description |
| ------------------- | ------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| children | ReactNode | Table header or data cells |
| id | string | A unique identifier for the row |
| selected? | boolean &#124; "indeterminate" | A boolean property indicating whether the row or it's related rows are selected |
| position | number | The zero-indexed position of the row. Used for Shift key multi-selection as well as selection of a range of rows when a `selectionRange` is set. |
| tone? | "subdued" &#124; "success" &#124; "warning" &#124; "critical" | Whether the row should visually indicate its status with a background color |
| disabled? | boolean | Whether the row should be disabled |
| selectionRange? | [number, number] | A tuple array with the first and last index of the range of other rows that the row describes. All non-disabled rows in the range are selected when the row with a selection range set is selected. |
| rowType? | "data" &#124; "subheader" | Indicates the relationship or role of the row's contents. A `rowType` of "subheader" looks and behaves the same as the table header. Defaults to "data". |
| accessibilityLabel? | string | Label set on the row's checkbox. Defaults to "Select \{resourceName\}" |
| onClick? | () => void | Callback fired when the row is clicked. Overrides the default click behaviour. |
| onNavigation? | (id: string) => void | Callback fired when the row is clicked and contains an anchor element with the `data-primary-link` property set |

## IndexTable.Cell

Expand Down Expand Up @@ -246,14 +244,14 @@ The `IndexTable` is an actionable, filterable, and sortable table widget that su

Merchants can select a group of rows at once by clicking or <kbd>Space</kbd> keypressing a subheader row's checkbox. To indicate that an `IndexTable.Row` serves as a subheader for 1 or more rows below it, set the:

- Zero-indexed table `position` of the first and last `IndexTable.Row` described by the subheader `IndexTable.Row` as a tuple array on its `subHeaderRange` prop
- Zero-indexed table `position` of the first and last `IndexTable.Row` described by the subheader `IndexTable.Row` as a tuple array on its `selectionRange` prop
- Unique `id` on the `IndexTable.Cell` that contains the subheader content
- Element tag to `"th"` on the `as` prop of the subheader `IndexTable.Cell`
- Subheader `IndexTable.Cell` `scope` prop to `"colgroup"`

To associate the subheader `IndexTable.Row` with each `IndexTable.Cell` that it describes, set the:

- Unique `id` provided to the subheader `IndexTable.Cell` on the `headers` prop of each related `IndexTable.Cell` (contained by an `IndexTable.Row` that's position is within the `subHeaderRange`) as well as the unique `id` of its corresponding column heading that you provided to the `IndexTable` `headings` prop
- Unique `id` provided to the subheader `IndexTable.Cell` on the `headers` prop of each related `IndexTable.Cell` (contained by an `IndexTable.Row` that's position is within the `selectionRange`) as well as the unique `id` of its corresponding column heading that you provided to the `IndexTable` `headings` prop

### Keyboard support

Expand Down
Loading

0 comments on commit c7c2312

Please sign in to comment.