[IndexTable] Fix checkboxes rendering when selectable=false #4376

brendanatshopify · 2021-08-10T21:17:05Z

WHY are these changes introduced?

Currently in the IndexTable component when the selectable prop is false the IndexTable remains selectable and the checkboxes still show.

This PR builds off of @chloerice's branch at #4367.

Fixes: #4076 and #4332

WHAT is this pull request doing?

This updates the IndexTable component to hide the checkboxes when the selectable prop is set to false. Any bulkActions and primaryBulkActions supplied to the component will also have no effect when selectable={false}.

Noteable changes

selectedItemsCount and onSelectionChange props are now optional in IndexProvider
When selectable={false} the Select button does not appear in the condensed view.
When selectable={true} the first two columns are sticky on tablet, and only the checkbox column is sticky on mobile. When selectable={false} only the first column is sticky in both screens.

Row clickability

If selectable={false} and the Row does not have data-primary-link the row will not respond to clicks. As such the cursor will be set to auto and any links in the row's cells will be individually clickable.

To make the entire row clickable to another url, one of the elements in the TableRow must be wrapped in an a tag with a data-primary-link attribute. Note that this is unchanged functionality.

Notes

There is some wonkiness in the sticky columns when scrolling left inside a table with a horizontal scrollbar. If a user scrolls left fast enough the sticky columns don't have the correct styles applied in time, which can sometimes produce an effect where the user can see the moving columns underneath the sticky columns. In addition, when moving left the header columns sometime lag behind the non-header columns in going left. This is also an issue on main but I wanted to call it out here in case this is something we want to address now/soon.

Videos

Unselectable - full screen	Selectable - full screen
Demo of a full screen IndexTable when selectable=false	Demo of a full screen IndexTable when selectable=true

Unselectable - with left scrollbar	Selectable - with left scrollbar
Demo of an IndexTable when selectable=false with a left scrollbar	Demo of an IndexTable when selectable=true with a left scrollbar

Unselectable - with left scrollbar (small screen)	Selectable - with left scrollbar (small screen)
Demo of an IndexTable when selectable=false with a left scrollbar on a small screen	Demo of an IndexTable when selectable=true with a left scrollbar on a small screen

Unselectable and condensed=true	Selectable and condensed=true
Demo of an IndexTable when selectable=false and condensed=true	Demo of an IndexTable when selectable=true and condensed=true

Demo of an IndexTable when selectable=false and rows don't have a data-primary-link

Demo of an IndexTable when selectable=false and rows have a data-primary-link

Demo of an IndexTable when selectable=false and rows don't have a data-primary-link but have an individual link

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx to test **unselectable**:

import React from 'react';

import {Card, IndexTable, Page, TextStyle} from '../src';

export function Playground() {
  const customers = [
    {
      id: '2563',
      url: 'customers/25630',
      name: 'Florence Jenkins',
      location: 'North Casandra',
      orders: 904,
      amountSpent: '$393',
    },
    {
      id: '2564',
      url: 'customers/25631',
      name: 'Anna Erdman MD',
      location: 'Kentmouth',
      orders: 620,
      amountSpent: '$471',
    },
    {
      id: '2565',
      url: 'customers/25632',
      name: 'Marvis Abbott',
      location: 'Port Ladawn',
      orders: 908,
      amountSpent: '$267',
    },
    {
      id: '2566',
      url: 'customers/25633',
      name: 'Keith Muller',
      location: 'South Mathew',
      orders: 581,
      amountSpent: '$705',
    },
    {
      id: '2567',
      url: 'customers/25634',
      name: 'Derrick Stracke',
      location: 'West Reneshire',
      orders: 400,
      amountSpent: '$905',
    },
    {
      id: '2568',
      url: 'customers/25635',
      name: 'Rupert Durgan',
      location: 'Lake Rhondabury',
      orders: 760,
      amountSpent: '$622',
    },
    {
      id: '2569',
      url: 'customers/25636',
      name: 'Lacy Kertzmann',
      location: 'Elinville',
      orders: 339,
      amountSpent: '$754',
    },
    {
      id: '2570',
      url: 'customers/25637',
      name: 'Haywood Heathcote',
      location: 'Port Delmar',
      orders: 191,
      amountSpent: '$599',
    },
    {
      id: '2571',
      url: 'customers/25638',
      name: 'Amb. Ayesha Bahringer',
      location: 'Ivoryfurt',
      orders: 332,
      amountSpent: '$752',
    },
    {
      id: '2572',
      url: 'customers/25639',
      name: 'Karl Goyette',
      location: 'Chayaland',
      orders: 869,
      amountSpent: '$947',
    },
    {
      id: '2573',
      url: 'customers/256310',
      name: 'Curtis Barton',
      location: 'South Harlanberg',
      orders: 196,
      amountSpent: '$543',
    },
    {
      id: '2574',
      url: 'customers/256311',
      name: 'Zetta Pouros',
      location: 'Janiemouth',
      orders: 424,
      amountSpent: '$373',
    },
    {
      id: '2575',
      url: 'customers/256312',
      name: 'Burton Corkery',
      location: 'Elizbethhaven',
      orders: 579,
      amountSpent: '$395',
    },
  ];

  const resourceName = {
    singular: 'customer',
    plural: 'customers',
  };

  const rowMarkup = customers.map(
    ({id, name, location, orders, amountSpent}, index) => {
      return (
        <IndexTable.Row id={id} key={id} position={index}>
          <IndexTable.Cell>
            <TextStyle variation="strong">{name}</TextStyle>
          </IndexTable.Cell>
          <IndexTable.Cell>{location}</IndexTable.Cell>
          <IndexTable.Cell>{orders}</IndexTable.Cell>
          <IndexTable.Cell>{amountSpent}</IndexTable.Cell>
        </IndexTable.Row>
      );
    },
  );

  return (
    <Page title="Playground">
      <Card>
        <IndexTable
          resourceName={resourceName}
          itemCount={customers.length}
          headings={[
            {title: 'Name'},
            {title: 'Location'},
            {title: 'Order count'},
            {title: 'Amount spent'},
          ]}
          selectable={false}
        >
          {rowMarkup}
        </IndexTable>
      </Card>
    </Page>
  );
}

Copy-paste this code in playground/Playground.tsx to test **selectable**:

import React from 'react';

import {Card, IndexTable, Page, TextStyle, useIndexResourceState} from '../src';

export function Playground() {
  const customers = [
    {
      id: '2563',
      url: 'customers/25630',
      name: 'Florence Jenkins',
      location: 'North Casandra',
      orders: 904,
      amountSpent: '$393',
    },
    {
      id: '2564',
      url: 'customers/25631',
      name: 'Anna Erdman MD',
      location: 'Kentmouth',
      orders: 620,
      amountSpent: '$471',
    },
    {
      id: '2565',
      url: 'customers/25632',
      name: 'Marvis Abbott',
      location: 'Port Ladawn',
      orders: 908,
      amountSpent: '$267',
    },
    {
      id: '2566',
      url: 'customers/25633',
      name: 'Keith Muller',
      location: 'South Mathew',
      orders: 581,
      amountSpent: '$705',
    },
    {
      id: '2567',
      url: 'customers/25634',
      name: 'Derrick Stracke',
      location: 'West Reneshire',
      orders: 400,
      amountSpent: '$905',
    },
    {
      id: '2568',
      url: 'customers/25635',
      name: 'Rupert Durgan',
      location: 'Lake Rhondabury',
      orders: 760,
      amountSpent: '$622',
    },
    {
      id: '2569',
      url: 'customers/25636',
      name: 'Lacy Kertzmann',
      location: 'Elinville',
      orders: 339,
      amountSpent: '$754',
    },
    {
      id: '2570',
      url: 'customers/25637',
      name: 'Haywood Heathcote',
      location: 'Port Delmar',
      orders: 191,
      amountSpent: '$599',
    },
    {
      id: '2571',
      url: 'customers/25638',
      name: 'Amb. Ayesha Bahringer',
      location: 'Ivoryfurt',
      orders: 332,
      amountSpent: '$752',
    },
    {
      id: '2572',
      url: 'customers/25639',
      name: 'Karl Goyette',
      location: 'Chayaland',
      orders: 869,
      amountSpent: '$947',
    },
    {
      id: '2573',
      url: 'customers/256310',
      name: 'Curtis Barton',
      location: 'South Harlanberg',
      orders: 196,
      amountSpent: '$543',
    },
    {
      id: '2574',
      url: 'customers/256311',
      name: 'Zetta Pouros',
      location: 'Janiemouth',
      orders: 424,
      amountSpent: '$373',
    },
    {
      id: '2575',
      url: 'customers/256312',
      name: 'Burton Corkery',
      location: 'Elizbethhaven',
      orders: 579,
      amountSpent: '$395',
    },
  ];

  
  const {
    selectedResources,
    allResourcesSelected,
    handleSelectionChange,
  } = useIndexResourceState(customers);

  const resourceName = {
    singular: 'customer',
    plural: 'customers',
  };

  const rowMarkup = customers.map(
    ({id, name, location, orders, amountSpent}, index) => {
      return (
        <IndexTable.Row
          id={id}
          key={id}
          position={index}
          selected={selectedResources.includes(id)}
        >
          <IndexTable.Cell>
            <TextStyle variation="strong">{name}</TextStyle>
          </IndexTable.Cell>
          <IndexTable.Cell>{location}</IndexTable.Cell>
          <IndexTable.Cell>{orders}</IndexTable.Cell>
          <IndexTable.Cell>{amountSpent}</IndexTable.Cell>
        </IndexTable.Row>
      );
    },
  );

  return (
    <Page title="Playground">
      <Card>
        <IndexTable
          resourceName={resourceName}
          itemCount={customers.length}
          selectedItemsCount={
            allResourcesSelected ? 'All' : selectedResources.length
          }
          onSelectionChange={handleSelectionChange}
          headings={[
            {title: 'Name'},
            {title: 'Location'},
            {title: 'Order count'},
            {title: 'Amount spent'},
          ]}
          selectable
        >
          {rowMarkup}
        </IndexTable>
      </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
For visual design changes, ping @ sarahill to update the Polaris UI kit

github-actions · 2021-08-11T13:17:42Z

size-limit report

Path	Size
cjs	143.11 KB (+0.2% 🔺)
esm	96.81 KB (+0.27% 🔺)
esnext	140.11 KB (+0.37% 🔺)
css	33.96 KB (+0.56% 🔺)

AndrewMusgrave · 2021-08-19T15:46:15Z

I wasn't aware the selectable prop existed! It was removed but must have accidentally been added while migrating the component from web -> Polaris.

Thanks for your contribution and for getting the API working ❤️ I'm removing myself as a reviewer since I'll be on vacation and won't be able to review.

lhoffbeck · 2021-08-23T13:35:32Z

There is some wonkiness in the sticky columns when scrolling left inside a table with a horizontal scrollbar. If a user scrolls left fast enough the sticky columns don't have the correct styles applied in time, which can sometimes produce an effect where the user can see the moving columns underneath the sticky columns. In addition, when moving left the header columns sometime lag behind the non-header columns in going left. This is also an issue on main but I wanted to call it out here in case this is something we want to address now/soon.

@brendanatshopify is there an issue for this already / if not could you add one and stick a link here? Sounds fun ;)

src/components/IndexTable/IndexTable.scss

src/components/IndexTable/IndexTable.tsx

lhoffbeck · 2021-08-23T14:05:16Z

src/components/IndexTable/IndexTable.tsx

    hasMoreLeftColumns && styles['Table-scrolling'],
    selectMode && styles.disableTextSelection,
    selectMode && shouldShowBulkActions && styles.selectMode,
+    isSelectableIndex === false && styles['Table-unselectable'],


[nit] for consistency with how it's used elsewhere, [c/sh]ould this be !isSelectableIndex?

*provided we're defaulting this value to true

Yup, updated, also renamed isSelectableIndex to selectable for better readability

src/components/IndexTable/IndexTable.tsx

lhoffbeck · 2021-08-23T14:09:48Z

src/components/IndexTable/components/Row/Row.tsx

-    hovered && styles['TableRow-hovered'],
-    status && styles[variationName('status', status)],
-  );
+  const getPrimaryLinkElement = () => {


[nit] this should maybe be memoized since (1) it's doing a DOM lookup, (2) it's called 3x on each rerender, and (3) the value isn't that likely to change on rerender

Good idea– it's a little tricky to memoize the function here since its dependencies aren't based on a state value, but I updated it to use a callback ref which will set primaryLinkElement.current only once when the tableRowRef mounts, which should have the same effect.

lhoffbeck · 2021-08-23T14:11:40Z

src/components/IndexTable/components/Row/Row.tsx

+      event.stopPropagation();
+      event.preventDefault();
+
+      const primaryLinkElement = getPrimaryLinkElement();


[nit] esp. if you don't memoize, this could be pulled above line 84 to remove a lookup

lhoffbeck · 2021-08-23T14:13:50Z

src/components/IndexTable/components/Row/tests/Row.test.tsx

    expect(onSelectionChangeSpy).toHaveBeenCalledTimes(1);
  });

+  it('does not fire onClick handler when row is clicked and no primary link child present and table is not selectable', () => {


[suggestion] could also add tests for checkbox is shown when selectable is true and not shown when selectable is false

lhoffbeck · 2021-08-23T14:24:08Z

src/components/IndexTable/tests/IndexTable.test.tsx

    });
  });

+  it('does not render checkboxes when selectable is set to false', () => {


Ahh I see, you're testing checkboxes here. There's an argument to be made for unit testing this Row checkbox functionality only within the Row component but it's more down to your testing philosophy 🤷

Agreed, moved this and added a couple more tests in Row.test.tsx to validate the checkbox presence

lhoffbeck · 2021-08-23T14:26:13Z

Mostly LGTM! Biggest concern is the hover color, everything else is nits. Ping me when I can 🎩 again!

pamelahicks · 2021-08-24T13:39:44Z

@brendanatshopify @lhoffbeck This might be a bit out of scope, but what do we think about making the fist column not sticky on small viewports when unselectable? I realize this table uses the pattern of leaving the first column sticky, but it seems a bit disastrous on the smaller viewports in cases where we need the table to be scrolling for whatever reason, as opposed to leveraging the condensed property:

first-col-sticky.mov

lhoffbeck · 2021-08-24T14:05:58Z

@pamelahicks 100% agree that's worth looking into in a different PR :) Do you know if there's a ticket already?

brendanatshopify · 2021-08-24T15:23:15Z

There is some wonkiness in the sticky columns when scrolling left inside a table with a horizontal scrollbar. If a user scrolls left fast enough the sticky columns don't have the correct styles applied in time, which can sometimes produce an effect where the user can see the moving columns underneath the sticky columns. In addition, when moving left the header columns sometime lag behind the non-header columns in going left. This is also an issue on main but I wanted to call it out here in case this is something we want to address now/soon.

@brendanatshopify is there an issue for this already / if not could you add one and stick a link here? Sounds fun ;)

Good call, issue opened at #4428

pamelahicks · 2021-08-25T13:52:30Z

@pamelahicks 100% agree that's worth looking into in a different PR :) Do you know if there's a ticket already?

Related to #4012

sandysameh · 2021-09-13T23:40:34Z

I want my row to be selectable but if i click on the button on my row it won't be selected and just toggles the button is that possible?

lhoffbeck · 2021-09-14T12:27:56Z

I want my row to be selectable but if i click on the button on my row it won't be selected and just toggles the button is that possible?

Hi @sandysameh! You should be able to do this by capturing the click event and using stopPropagation() to prevent it from bubbling up to the row's handler.

danielkcz · 2021-10-07T07:15:39Z

Any ETA on when this is going to be published?

lhoffbeck · 2021-10-07T12:13:48Z

Hi @FredyC, it's out as of the v7 release :)

This was referenced Aug 10, 2021

Shop channel fix selectable index table #4368

Closed

[IndexTable] Add support for non-selectable table when selectable prop is false #4329

Closed

brendanatshopify changed the title ~~Shop channel/fix indextable selectable~~ [IndexTable] Shop channel/fix indextable selectable Aug 10, 2021

brendanatshopify force-pushed the shop-channel/fix-indextable-selectable branch from 56a2724 to 686059a Compare August 17, 2021 15:16

brendanatshopify marked this pull request as ready for review August 17, 2021 21:21

brendanatshopify changed the title ~~[IndexTable] Shop channel/fix indextable selectable~~ [IndexTable] Fix checkboxes rendering when selectable=false Aug 17, 2021

brendanatshopify requested review from AndrewMusgrave, lhoffbeck and pamelahicks August 17, 2021 21:22

AndrewMusgrave removed their request for review August 19, 2021 15:46