Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IndexTable] Merge IndexTable accessibility change from web #4004

Merged
merged 1 commit into from
Feb 17, 2021

Conversation

lhoffbeck
Copy link
Contributor

@lhoffbeck lhoffbeck commented Feb 17, 2021

WHY are these changes introduced?

The IndexTable sticky header's bulk action checkbox can "steal" tab focus while still hidden due to it appearing higher in the dom than the non-sticky bulk action checkbox. This change was already merged in shopify/web with this pr, migrating the change here as well.

WHAT is this pull request doing?

This PR positions the sticky header out of the viewport so that the focus utility properly skips the hidden checkbox.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

In the playground, open the More actions dropdown via the keyboard and tab through the dropdown options until it closes. With my changes, the next focused element after closing the dropdown is the select all checkbox.

Copy-paste this code in playground/Playground.tsx:
import React, {useCallback, useState} from 'react';

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

export function Playground() {
  return (
    <Page title="Playground">
      <PopoverWithActionListExample />
      <IndexTableWithBulkActionsExample />
      {/* Add the code you want to test in here */}
    </Page>
  );
}

function PopoverWithActionListExample() {
  const [popoverActive, setPopoverActive] = useState(true);

  const togglePopoverActive = useCallback(
    () => setPopoverActive((popoverActive) => !popoverActive),
    [],
  );

  const activator = (
    <Button onClick={togglePopoverActive} disclosure>
      More actions
    </Button>
  );

  return (
    <div style={{height: '250px'}}>
      <Popover
        active={popoverActive}
        activator={activator}
        onClose={togglePopoverActive}
      >
        <ActionList items={[{content: 'Import'}, {content: 'Export'}]} />
      </Popover>
    </div>
  );
}

function IndexTableWithBulkActionsExample() {
  const customers = [
    {
      id: '3413',
      url: 'customers/341',
      name: 'Mae Jemison',
      location: 'Decatur, USA',
      orders: 20,
      amountSpent: '$2,400',
    },
    {
      id: '2563',
      url: 'customers/256',
      name: 'Ellen Ochoa',
      location: 'Los Angeles, USA',
      orders: 30,
      amountSpent: '$140',
    },
  ];
  const resourceName = {
    singular: 'customer',
    plural: 'customers',
  };

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

  const promotedBulkActions = [
    {
      content: 'Edit customers',
      onAction: () => console.log('Todo: implement bulk edit'),
    },
  ];
  const bulkActions = [
    {
      content: 'Add tags',
      onAction: () => console.log('Todo: implement bulk add tags'),
    },
    {
      content: 'Remove tags',
      onAction: () => console.log('Todo: implement bulk remove tags'),
    },
    {
      content: 'Delete customers',
      onAction: () => console.log('Todo: implement bulk delete'),
    },
  ];

  const rowMarkup = customers.map(
    ({id, name, location, orders, amountSpent}, index) => (
      <IndexTable.Row
        id={id}
        key={id}
        selected={selectedResources.includes(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 (
    <Card>
      <IndexTable
        resourceName={resourceName}
        itemCount={customers.length}
        selectedItemsCount={
          allResourcesSelected ? 'All' : selectedResources.length
        }
        onSelectionChange={handleSelectionChange}
        bulkActions={bulkActions}
        promotedBulkActions={promotedBulkActions}
        headings={[
          {title: 'Name'},
          {title: 'Location'},
          {title: 'Order count'},
          {title: 'Amount spent'},
        ]}
      >
        {rowMarkup}
      </IndexTable>
    </Card>
  );
}

🎩 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, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@github-actions
Copy link
Contributor

github-actions bot commented Feb 17, 2021

🟢 This pull request modifies 2 files and might impact 1 other files.

Details:
All files potentially affected (total: 1)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/IndexTable/IndexTable.scss (total: 1)

Files potentially affected (total: 1)

Copy link
Member

@AndrewMusgrave AndrewMusgrave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't 🎩 since this change was merged into web. Thank you for bringing it over 🙇 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants