[DataTable] Add fixedFirstColumns prop

[tylernoseworthy]

WHY are these changes introduced?

We are currently using the DataTable, but there was a requirement in our current project for the first 2 columns within the table to be fixed.

WHAT is this pull request doing?

This PR adds a fixFirstColumns prop in favour of the hasFixedFirstColumn prop, which now has a deprecation warning if still being used in development. In the event that hasFixedFirstColumn is still being used, we will default to showing 1 fixed column. Additionally, if a number defined in fixFirstColumns exceeds or is equal to the amount of columns within the table, no fixed columns will be rendered.

Before

After

Without sticky header

With sticky header

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

import React from 'react';
import {Page, Card, DataTable} from '../src';

export function Playground() {
  return (
    const rows = [
    [
      'Emerald Silk Gown',
      'Formalwear',
      "Jill's formal",
      10,
      '$875.00',
      124689,
      140,
      '$426.00',
      '$122,500.00',
    ],
    [
      'Mauve Cashmere Scarf',
      'Accessories',
      "Jack's Accessories",
      253,
      '$230.00',
      124533,
      83,
      '$620.00',
      '$19,090.00',
    ],
    [
      'Navy Merino Wool Blazer with khaki chinos and yellow belt',
      'Ensembles',
      'Avocado Fashions',
      23,
      '$445.00',
      124518,
      32,
      '$353.00',
      '$14,240.00',
    ],
    [
      'Socks',
      'Essentials',
      'Avocado Fashions',
      465,
      '$4.00',
      124518,
      32,
      '$3.00',
      '$140.00',
    ],
  ];

  return (
    <Page title="Sales by product">
      <Card>
        <DataTable
          columnContentTypes={[
            'text',
            'text',
            'text',
            'numeric',
            'numeric',
            'numeric',
            'numeric',
            'numeric',
            'numeric',
          ]}
          headings={[
            'Product',
            'Category',
            'Vendor',
            'Orders',
            'Price',
            'SKU Number',
            'Net quantity',
            'Shipping',
            'Net sales',
          ]}
          rows={rows}
          totals={['', '', '', '', '', '', 255, '$1399', '$155,830.00']}
          initialSortColumnIndex={4}
          stickyHeader
          fixedFirstColumns={3}
          truncate
          showTotalsInFooter
          footerContent={`Showing ${rows.length} of ${rows.length} results`}
        />
      </Card>
    </Page>
  );
}

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• Tophatted documentation changes in the style guide

[tylernoseworthy]

/snapit

[github-actions]

size-limit report 📦

Path	Size
polaris-react-cjs	200.65 KB (+0.1% 🔺)
polaris-react-esm	128.92 KB (+0.15% 🔺)
polaris-react-esnext	183.02 KB (+0.11% 🔺)
polaris-react-css	40.6 KB (+0.01% 🔺)

[tylernoseworthy]

/snapit

[nickpresta]

/snapit

[nickpresta]

We also wanted to fix these examples since the sorting doesn't really work.

[nickpresta]

/snapit

[nickpresta]

/snapit

[github-actions]

🫰✨ Thanks @nickpresta! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris@0.0.0-snapshot-release-20220818200014

yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20220818200014

yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20220818200014

[nickpresta]

I left comments on the stories that changed in Chromatic -- it would be great for someone else to take a look and verify my comments and accept the changes.

[tylernoseworthy]

Wondering if this should just be called fixed or fixedColumn

[ryanpdaley]

Looks good to me.

[tylernoseworthy]

Just leaving a comment about a UI issue that potentially could happen if you pass a number of fixedFirstColumns that exceeds the table width. This causes there to be an overflow of the table which hides the scrollable table. I am thinking this is an edge case that should be handled by who ever uses the component, seeing as having 2+ fixed columns on certain screen sizes isn't the greatest experience.

[chloerice]

Hey @tylernoseworthy 👋🏽 This works great on large screen but needs to be limited to 1 fixed column (or none at all) on small screens. Currently the table breaks on small screens:

[chloerice]

Suggested change

	Added `fixedFirstColumns` prop to `DataTable` so that multiple columns can be fixed
	- Added a `fixedFirstColumns` prop to `DataTable` so that multiple columns can be fixed
	- Deprecated the `DataTable` `fixedFirstColumn` prop

[sam-b-rose]

Looks like this caused a bit of a kerfuffle in the generated changelog. Can we reduce this to a single line without bullets? Not as fancy, but might improve how changesets renders it

[tylernoseworthy]

@samrose3 Am I able to make that change directly on that branch? or is there a special command I have to run?

[sam-b-rose]

You can just modify the changeset file directly (.changeset/lazy-nails-relax.md) no command needed 👍

[nickpresta]

@chloerice Thanks for the feedback! I'd like to understand a bit better -- is there a reason this needs to be baked into Polaris? The fixedFirstColumns prop is configurable, and the default state is to have no fixed columns, which means by default things are fine on small screens, and any consumer opting into fixedFirstColumns behavior could handle the responsiveness themselves.

We don't seem to take a stance of overriding props when certain combinations wouldn't look great on small screens - for example grid, button group, index table require the consumer to make an explicit choice about how to handle small screens.

For the data table, it would mean changing the fixedFirstColumns prop to something like 0 or 1 on small screens.

Thanks for any context!

[chloerice]

@chloerice Thanks for the feedback! I'd like to understand a bit better -- is there a reason this needs to be baked into Polaris? The fixedFirstColumns prop is configurable, and the default state is to have no fixed columns, which means by default things are fine on small screens, and any consumer opting into fixedFirstColumns behavior could handle the responsiveness themselves.

We don't seem to take a stance of overriding props when certain combinations wouldn't look great on small screens - for example grid, button group, index table require the consumer to make an explicit choice about how to handle small screens.

For the data table, it would mean changing the fixedFirstColumns prop to something like 0 or 1 on small screens.

Thanks for any context!

Hey @nickpresta! I think that approach of putting the onus on consumers is fine, as long the documentation for that prop calls it out and its examples show how to implement it.

[tylernoseworthy]

I noticed this will not work when editing in codesandbox because of the useMedia hook. Is this something we should be worried about?

[chloerice]

Hey @tylernoseworthy you'll need to copy and paste the hook into the bottom of the example instead of importing it.

[tylernoseworthy]

Forgot to do it before merging 😅. I'll follow up with a another PR to fix the example for codesandbox.