Skip to content

Commit 60086a6

Browse files
authored
[DataTable] Better prop compatibility and fixing no-wrap issue. (#7043)
### WHY are these changes introduced? @brianshen1990 was integrating the new `DataTable` in the `BulkAccountInviter` app and noticed that regular cells were never wrapping in multiple lines, pushing the table width beyond the available space. This triggered the condensed nav. This PR fixes this by defaulting to cells wrapping unless the `truncate` prop is specifically set. This was caused by [this PR](#6914) which wrapped all cells in a `TruncatedText` component which would automatically add tooltips in case content doesn't have enough space. This was a bit too heavy handed. After this PR `TruncatedText` is only applied if the `truncate` prop is specifically set. I also ensured the fixed first column feature will work properly no matter whether `truncated` is set or not. This means you can now have fixed first columns with text wrapping and with text being truncated depending on what the consumer prefers. Lastly, there seemed to be a bug in the calculation of headers when `fixedFirstColums` is set to a value larger than 1. This was because currently we're only calculating `columnVisibilityData` when `condensed` was `true`. This means no heading widths are available to properly calculate the absolute positioning of fixed headings in the sticky header unless the viewport width was reduced to trigger `condensed` to be `true`. After this PR `columnVisibilityData` will be calculated when either `stickyHeading` is `true` or the table is `condensed`. <details> <summary>Screenshot of wrapping issue</summary> ![Condensed navigation due to text in first column not wrapping](https://user-images.githubusercontent.com/4960217/187224210-139a212e-564c-4005-bf34-b58e15f8d6a5.png) </details> <details> <summary>Video of heading with calculation issue</summary> ![Sticky heading calculated ionco](https://user-images.githubusercontent.com/4960217/187226773-183ff585-c21b-4312-98e3-e22b894944ac.gif) </details> ### WHAT is this pull request doing? <details> <summary>GIF of sticky heading width calculated correctly</summary> ![GIF of correct heading calculation](https://user-images.githubusercontent.com/4960217/187227214-f5e8d87b-1407-4609-a539-3007e1ce55e9.gif) </details> <details> <summary>GIF of truncation with and without fixed first column</summary> ![Truncation in combination with fixed first column and without](https://user-images.githubusercontent.com/4960217/187227738-5ca06194-2c52-4f13-8da6-2ac328ed9219.gif) </details> ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) **Spin-URL: (`shopify/web`)** https://shop1.shopify.data-table-enhancements.philipp-schofer.us.spin.dev/admin/ 1. Try combinations of the following props: - `truncate` on/off - `fixedFirstColumns` - 1,2,3 or 0 - `stickyHeader` on/off 2. Open the Playground in Safari and ensure everything renders correctly when the above props are combined. <details> <summary>Copy-paste this code in <code>playground/Playground.tsx</code>:</summary> ```jsx import {Page, Card, DataTable} from '@shopify/polaris'; import React from 'react'; export function Playground() { const rows = [ ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ['Emerald Silk Gown', '$875.00', 124689, 140, '$122,500.00'], ['Mauve Cashmere Scarf', '$230.00', 124533, 83, '$19,090.00'], [ 'Navy Merino Wool Blazer with khaki chinos and yellow belt', '$445.00', 124518, 32, '$14,240.00', ], ]; return ( <> <Page title="Sales by product"> <Card> <DataTable columnContentTypes={[ 'text', 'numeric', 'numeric', 'numeric', 'numeric', ]} headings={[ 'Product', 'Price', 'SKU Number', 'Net quantity', 'Net sales', ]} rows={rows} // increasedTableDensity // truncate stickyHeader showTotalsInFooter // fixedFirstColumns={2} totals={['', '', '', 255, '$155,830.00']} /> </Card> </Page> <br /> </> ); } ``` </details> ### 🎩 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) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide
1 parent 7a548a0 commit 60086a6

File tree

4 files changed

+74
-35
lines changed

4 files changed

+74
-35
lines changed

.changeset/mean-rats-prove.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
'@shopify/polaris': minor
3+
---
4+
5+
Updates to `DataTable`
6+
7+
- Fixed `DataTable` cell content not wrapping when the `truncate` prop is `false`
8+
- Added support for setting the `DataTable` `truncate` prop without having to set the `fixedFirstColumns` prop

polaris-react/src/components/DataTable/DataTable.scss

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
--pc-data-table-first-column-width: 145px;
55
position: relative;
66
max-width: 100vw;
7+
background-color: var(--p-surface);
8+
border-radius: var(--p-border-radius-2);
9+
overflow: hidden;
710
}
811

912
.condensed {
@@ -47,6 +50,7 @@
4750
overflow-x: auto;
4851
-webkit-overflow-scrolling: touch;
4952
scroll-behavior: smooth;
53+
background-color: inherit;
5054
}
5155

5256
.Table {
@@ -62,6 +66,7 @@
6266
}
6367

6468
.Cell {
69+
@include text-emphasis-normal;
6570
padding: var(--p-space-4);
6671
white-space: nowrap;
6772
text-align: left;
@@ -88,8 +93,16 @@
8893
}
8994
}
9095

96+
.Cell-separate::after {
97+
content: '';
98+
position: absolute;
99+
top: 0;
100+
right: 0;
101+
bottom: 0;
102+
border-right: var(--p-border-divider);
103+
}
104+
91105
.Cell-firstColumn {
92-
@include text-emphasis-normal;
93106
text-align: left;
94107
white-space: normal;
95108
}
@@ -319,6 +332,11 @@
319332
height: 0;
320333
width: 0;
321334
}
335+
336+
.FixedFirstColumn {
337+
bottom: 0;
338+
top: auto;
339+
}
322340
}
323341

324342
.StickyTableColumnHeader-isScrolling {
@@ -339,20 +357,15 @@
339357

340358
.FixedFirstColumn {
341359
position: absolute;
342-
background: var(--p-surface);
360+
background: inherit;
343361
z-index: 3;
344362
border-spacing: 0;
363+
top: 0;
345364
left: 0;
365+
346366
@media #{$p-breakpoints-md-down} {
347367
z-index: 1;
348368
}
349-
350-
&.separate:is(table) th,
351-
&.separate:is(th) {
352-
&:last-of-type {
353-
border-right: var(--p-border-divider);
354-
}
355-
}
356369
}
357370

358371
.TooltipContent {

polaris-react/src/components/DataTable/DataTable.tsx

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
136136
let condensed = false;
137137

138138
if (table && scrollContainer) {
139-
condensed = table.scrollWidth > scrollContainer.clientWidth;
139+
// safari sometimes incorrectly sets the scrollwidth too large by 1px
140+
condensed = table.scrollWidth > scrollContainer.clientWidth + 1;
140141
}
141142

142143
this.setState({
@@ -246,9 +247,7 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
246247
!isScrolledFarthestLeft && styles.separate,
247248
)}
248249
style={{
249-
maxWidth: `${
250-
columnVisibilityData[fixedFirstColumns - 1]?.rightEdge
251-
}px`,
250+
width: `${columnVisibilityData[fixedFirstColumns - 1]?.rightEdge}px`,
252251
}}
253252
>
254253
<thead>
@@ -293,7 +292,11 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
293292
);
294293

295294
const bodyMarkup = rows.map((row, index) =>
296-
this.defaultRenderRow({row, index, inFixedNthColumn: false}),
295+
this.defaultRenderRow({
296+
row,
297+
index,
298+
inFixedNthColumn: false,
299+
}),
297300
);
298301

299302
const footerMarkup = footerContent ? (
@@ -438,7 +441,7 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
438441
button.addEventListener('focus', this.handleHeaderButtonFocus);
439442
} else {
440443
this.tableHeadings[index] = ref;
441-
this.tableHeadingWidths[index] = ref.getBoundingClientRect().width;
444+
this.tableHeadingWidths[index] = ref.clientWidth;
442445
}
443446
};
444447

@@ -511,8 +514,9 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
511514
scrollContainer: {current: scrollContainer},
512515
dataTable: {current: dataTable},
513516
} = this;
517+
const {stickyHeader} = this.props;
514518

515-
if (condensed && table && scrollContainer && dataTable) {
519+
if ((stickyHeader || condensed) && table && scrollContainer && dataTable) {
516520
const headerCells = table.querySelectorAll<HTMLTableCellElement>(
517521
headerCell.selector,
518522
);
@@ -758,7 +762,9 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
758762
content: heading,
759763
contentType: columnContentTypes[headingIndex],
760764
nthColumn: headingIndex < fixedFirstColumns,
765+
fixedFirstColumns,
761766
truncate,
767+
headingIndex,
762768
...sortableHeadingProps,
763769
verticalAlign,
764770
handleFocus: this.handleFocus,
@@ -795,13 +801,10 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
795801
inStickyHeader,
796802
});
797803
}}
798-
inFixedNthColumn
804+
inFixedNthColumn={Boolean(fixedFirstColumns)}
805+
lastFixedFirstColumn={headingIndex === fixedFirstColumns - 1}
799806
style={{
800807
left: this.state.columnVisibilityData[headingIndex]?.leftEdge,
801-
borderRight:
802-
headingIndex === fixedFirstColumns - 1 && fixedCellVisible
803-
? 'var(--p-border-divider)'
804-
: undefined,
805808
}}
806809
/>,
807810
];
@@ -818,6 +821,7 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
818821
inStickyHeader,
819822
});
820823
}}
824+
lastFixedFirstColumn={headingIndex === fixedFirstColumns - 1}
821825
inFixedNthColumn={inFixedNthColumn}
822826
/>
823827
);
@@ -869,6 +873,7 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
869873
total
870874
totalInFooter={totalInFooter}
871875
nthColumn={index <= fixedFirstColumns - 1}
876+
firstColumn={index === 0}
872877
key={id}
873878
content={content}
874879
contentType={contentType}
@@ -914,6 +919,7 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
914919
hoverable = true,
915920
headings,
916921
} = this.props;
922+
const {condensed} = this.state;
917923
const fixedFirstColumns = this.fixedFirstColumns();
918924
const className = classNames(
919925
styles.TableRow,
@@ -926,7 +932,6 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
926932
className={className}
927933
onMouseEnter={this.handleHover(index)}
928934
onMouseLeave={this.handleHover()}
929-
style={rowHeights ? {height: `${rowHeights[index]}px`} : {}}
930935
>
931936
{row.map((content: CellProps['content'], cellIndex: number) => {
932937
const hovered = index === this.state.rowHovered;
@@ -944,11 +949,13 @@ class DataTableInner extends PureComponent<CombinedProps, DataTableState> {
944949
content={content}
945950
contentType={columnContentTypes[cellIndex]}
946951
nthColumn={cellIndex <= fixedFirstColumns - 1}
952+
firstColumn={cellIndex === 0}
947953
truncate={truncate}
948954
verticalAlign={verticalAlign}
949955
colSpan={colSpan}
950956
hovered={hovered}
951-
inFixedNthColumn={inFixedNthColumn}
957+
style={rowHeights ? {height: `${rowHeights[index]}px`} : {}}
958+
inFixedNthColumn={condensed && inFixedNthColumn}
952959
/>
953960
);
954961
})}

polaris-react/src/components/DataTable/components/Cell/Cell.tsx

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface CellProps {
1313
content?: React.ReactNode;
1414
contentType?: string;
1515
nthColumn?: boolean;
16+
firstColumn?: boolean;
1617
truncate?: boolean;
1718
header?: boolean;
1819
total?: boolean;
@@ -34,12 +35,14 @@ export interface CellProps {
3435
fixedCellVisible?: boolean;
3536
firstColumnMinWidth?: string;
3637
style?: React.CSSProperties;
38+
lastFixedFirstColumn?: boolean;
3739
}
3840

3941
export function Cell({
4042
content,
4143
contentType,
4244
nthColumn,
45+
firstColumn,
4346
truncate,
4447
header,
4548
total,
@@ -61,15 +64,16 @@ export function Cell({
6164
fixedCellVisible = false,
6265
firstColumnMinWidth,
6366
style,
67+
lastFixedFirstColumn,
6468
}: CellProps) {
6569
const i18n = useI18n();
6670
const numeric = contentType === 'numeric';
6771

6872
const className = classNames(
6973
styles.Cell,
7074
styles[`Cell-${variationName('verticalAlign', verticalAlign)}`],
71-
nthColumn && styles['Cell-firstColumn'],
72-
nthColumn && truncate && styles['Cell-truncated'],
75+
firstColumn && styles['Cell-firstColumn'],
76+
truncate && styles['Cell-truncated'],
7377
header && styles['Cell-header'],
7478
total && styles['Cell-total'],
7579
totalInFooter && styles['Cell-total-footer'],
@@ -78,7 +82,10 @@ export function Cell({
7882
sorted && styles['Cell-sorted'],
7983
stickyHeadingCell && styles.StickyHeaderCell,
8084
hovered && styles['Cell-hovered'],
81-
fixedCellVisible && styles.separate,
85+
lastFixedFirstColumn &&
86+
inFixedNthColumn &&
87+
fixedCellVisible &&
88+
styles['Cell-separate'],
8289
nthColumn &&
8390
inFixedNthColumn &&
8491
stickyHeadingCell &&
@@ -156,31 +163,35 @@ export function Cell({
156163
const headingMarkup = header ? (
157164
<th
158165
{...headerCell.props}
166+
aria-sort={sortDirection}
159167
{...colSpanProp}
160168
ref={setRef}
161169
className={className}
162170
scope="col"
163-
aria-sort={sortDirection}
164-
style={nthColumn ? {minWidth: firstColumnMinWidth} : {}}
171+
style={{...minWidthStyles}}
165172
>
166173
{columnHeadingContent}
167174
</th>
168175
) : (
169176
<th
170-
style={{minWidth: firstColumnMinWidth}}
177+
{...colSpanProp}
178+
ref={setRef}
171179
className={className}
172180
scope="row"
173-
{...colSpanProp}
174-
ref={(ref) => {
175-
setRef(ref);
176-
}}
181+
style={{...minWidthStyles}}
177182
>
178-
<TruncatedText className={styles.TooltipContent}>{content}</TruncatedText>
183+
{truncate ? (
184+
<TruncatedText className={styles.TooltipContent}>
185+
{content}
186+
</TruncatedText>
187+
) : (
188+
content
189+
)}
179190
</th>
180191
);
181192

182193
const cellMarkup =
183-
header || nthColumn ? (
194+
header || firstColumn || nthColumn ? (
184195
headingMarkup
185196
) : (
186197
<td className={className} {...colSpanProp}>

0 commit comments

Comments
 (0)