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

[ResourceList] Multiselect #1261

Merged
merged 2 commits into from
May 29, 2019
Merged

[ResourceList] Multiselect #1261

merged 2 commits into from
May 29, 2019

Conversation

AndrewMusgrave
Copy link
Member

@AndrewMusgrave AndrewMusgrave commented Mar 28, 2019

WHY are these changes introduced?

Add multi-select to ResourceList

Fixes: #935

WHAT is this pull request doing?

  • Add example to readme
  • Add multiselect logic
  • Add tests
  • Fix failing tests

Video

https://cl.ly/9e5e755caa40

How to 🎩

  • Playground with multiselect is provided below
  • I'm linking to an example PR in web adding multiselect that can be used for 🎩 (hidden between the deploys)
Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {Card, ResourceList, Avatar, TextStyle} from '../src';

const items = [
  {
    id: 341,
    url: 'customers/341',
    name: 'Mae Jemison',
    location: 'Decatur, USA',
  },
  {
    id: 256,
    url: 'customers/256',
    name: 'Ellen Ochoa',
    location: 'Los Angeles, USA',
  },
  {
    id: 3412341,
    url: 'customers/341',
    name: 'Mae Jemison',
    location: 'Decatur, USA',
  },
  {
    id: 25566546,
    url: 'customers/256',
    name: 'Ellen Ochoa',
    location: 'Los Angeles, USA',
  },
  {
    id: 34222341,
    url: 'customers/341',
    name: 'Mae Jemison',
    location: 'Decatur, USA',
  },
  {
    id: 2567896,
    url: 'customers/256',
    name: 'Ellen Ochoa',
    location: 'Los Angeles, USA',
  },
  {
    id: 34765451,
    url: 'customers/341',
    name: 'Mae Jemison',
    location: 'Decatur, USA',
  },
  {
    id: 2543436,
    url: 'customers/256',
    name: 'Ellen Ochoa',
    location: 'Los Angeles, USA',
  },
  {
    id: 312341,
    url: 'customers/341',
    name: 'Mae Jemison',
    location: 'Decatur, USA',
  },
  {
    id: 434256,
    url: 'customers/256',
    name: 'Ellen Ochoa',
    location: 'Los Angeles, USA',
  },
];

export default class ResourceListExample extends React.Component {
  state = {
    selectedItems: [],
  };

  handleSelectionChange = (selectedItems) => {
    this.setState({selectedItems});
  };

  renderItem = (item, _, index) => {
    const {id, url, name, location} = item;
    const media = <Avatar customer size="medium" name={name} />;

    return (
      <ResourceList.Item
        id={id}
        url={url}
        media={media}
        sortOrder={index}
        accessibilityLabel={`View details for ${name}`}
      >
        <h3>
          <TextStyle variation="strong">{name}</TextStyle>
        </h3>
        <div>{location}</div>
      </ResourceList.Item>
    );
  };

  render() {
    const resourceName = {
      singular: 'customer',
      plural: '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'),
      },
    ];

    return (
      <Card>
        <ResourceList
          resourceName={resourceName}
          items={items}
          renderItem={this.renderItem}
          selectedItems={this.state.selectedItems}
          onSelectionChange={this.handleSelectionChange}
          promotedBulkActions={promotedBulkActions}
          bulkActions={bulkActions}
          resolveItemId={resolveItemIds}
        />
      </Card>
    );
  }
}

function resolveItemIds({id}) {
  return id;
}

@BPScott BPScott temporarily deployed to polaris-react-pr-1261 March 28, 2019 15:49 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1261 March 28, 2019 15:51 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1261 March 29, 2019 15:05 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1261 April 2, 2019 17:28 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1261 April 2, 2019 17:38 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1261 April 2, 2019 18:18 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1261 April 2, 2019 19:09 Inactive
@AndrewMusgrave AndrewMusgrave changed the title [WIP][ResourceList] Multiselect [ResourceList] Multiselect Apr 2, 2019
@danrosenthal
Copy link
Contributor

Re-pinging @alex-page and @dleroux for this one, as it's been quiet for a little while

@dleroux
Copy link
Contributor

dleroux commented May 13, 2019

Sorry for the delay on this. Do you mind rebasing and pinging me again @AndrewMusgrave ?

@AndrewMusgrave
Copy link
Member Author

@dleroux

@@ -533,6 +537,7 @@ export class ResourceList extends React.Component<CombinedProps, State> {
const resourceListClassName = classNames(
styles.ResourceList,
loading && styles.disabledPointerEvents,
selectMode && styles.disableTextSelection,
Copy link
Contributor

Choose a reason for hiding this comment

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

what was happening without this?

Copy link
Member Author

Choose a reason for hiding this comment

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

You'll select text when multi-selecting, really bad experience 😬

checked={selected}
disabled={loading}
/>
<span onChange={this.handleLargerSelectionArea}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Our Checkbox renders block elements so probably best to make this a div.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch, I'll change this on Monday and merge 🚢

Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

🎩 looks good. Just a small change request on the mark-up, after that 🐑 🇮🇹

@AndrewMusgrave
Copy link
Member Author

Before I 🚢 , @alex-page what do you think?

Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

Great work @AndrewMusgrave, make sure the id's on the items are unique compared to the other examples otherwise you will get an accessibility issue.

Change div to span for accessible markup
@AndrewMusgrave AndrewMusgrave merged commit a9188f1 into master May 29, 2019
@AndrewMusgrave AndrewMusgrave deleted the rl-ms branch May 29, 2019 15:33
@dleroux dleroux temporarily deployed to production June 5, 2019 14:15 Inactive
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.

[ResourceList] support shift-key range selections
5 participants