Skip to content

Conversation

alex-page
Copy link
Member

@alex-page alex-page commented Mar 13, 2019

WHY are these changes introduced?

Resolves #1179
Resolves https://github.com/Shopify/web/issues/11580

WHAT is this pull request doing?

This pull request stops onClick firing three times. The three events happened because:

  • Enter key fires onClick and keyUp handler
  • Stop propogation was never onClick
  • The onClick was only meant to be fired when the resource list item had an anchor tag

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

The below code can be tested by tabbing to the first item which is an anchor and making sure that on enter key press the number increases in the title. The second item should also only increment the amount by one when key pressing enter.

Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {
  Avatar,
  Card,
  Label,
  FilterType,
  List,
  ResourceList,
  TextStyle,
} from '../src';

interface State {}

export default class ResourceListExample extends React.Component {
  state = {
    selectedItems: [],
    sortValue: 'DATE_MODIFIED_DESC',
    searchValue: '',
    appliedFilters: [
      {
        key: 'accountStatusFilter',
        value: 'Account enabled',
      },
    ],
    clicked: 0,
  };

  handleSearchChange = (searchValue) => {
    this.setState({searchValue});
  };

  handleFiltersChange = (appliedFilters) => {
    this.setState({appliedFilters});
  };

  handleSortChange = (sortValue) => {
    this.setState({sortValue});
  };

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

  handleClick = () => {
    this.setState(({clicked}) => ({clicked: clicked + 1}));
  };

  renderItem = (item) => {
    const {id, url, name, location, latestOrderUrl} = item;
    const media = <Avatar customer size="medium" name={name} />;
    const shortcutActions = latestOrderUrl
      ? [{content: 'View latest order', url: latestOrderUrl}]
      : null;
    return (
      <ResourceList.Item
        id={id}
        url={url}
        media={media}
        accessibilityLabel={`View details for ${name}`}     
        shortcutActions={shortcutActions}
        onClick={this.handleClick}
        persistActions
      >
        <h3>
          <TextStyle variation="strong">
            {name} <span>{this.state.clicked}</span>
          </TextStyle>
        </h3>
        <div>{location}</div>
      </ResourceList.Item>
    );
  };

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

    const items = [
      {
        url: '#abc',
        id: 341,
        name: 'Mae Jemison',
        location: 'Decatur, USA',
        latestOrderUrl: 'orders/1456',
      },
      {
        id: 256,
        name: 'Ellen Ochoa',
        location: 'Los Angeles, USA',
        latestOrderUrl: 'orders/1457',
      },
    ];

    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 filters = [
      {
        key: 'orderCountFilter',
        label: 'Number of orders',
        operatorText: 'is greater than',
        type: FilterType.TextField,
      },
      {
        key: 'accountStatusFilter',
        label: 'Account status',
        operatorText: 'is',
        type: FilterType.Select,
        options: ['Enabled', 'Invited', 'Not invited', 'Declined'],
      },
    ];

    const filterControl = (
      <ResourceList.FilterControl
        filters={filters}
        appliedFilters={this.state.appliedFilters}
        onFiltersChange={this.handleFiltersChange}
        searchValue={this.state.searchValue}
        onSearchChange={this.handleSearchChange}
        additionalAction={{
          content: 'Save',
          onAction: () => console.log('New filter saved'),
        }}
      />
    );

    return (
      <Card>
        <ResourceList
          resourceName={resourceName}
          items={items}
          renderItem={this.renderItem}
          selectedItems={this.state.selectedItems}
          onSelectionChange={this.handleSelectionChange}
          promotedBulkActions={promotedBulkActions}
          bulkActions={bulkActions}
          sortValue={this.state.sortValue}
          sortOptions={[
            {label: 'Newest update', value: 'DATE_MODIFIED_DESC'},
            {label: 'Oldest update', value: 'DATE_MODIFIED_ASC'},
          ]}
          onSortChange={(selected) => {
            this.setState({sortValue: selected});
            console.log(`Sort option changed to ${selected}.`);
          }}
          filterControl={filterControl}
        />
      </Card>
    );
  }
}

🎩 checklist

@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 13, 2019 19:05 Inactive
@alex-page alex-page requested a review from chloerice March 13, 2019 19:05
@alex-page alex-page changed the title ResourceList: onClick is called three times on keypress of Enter [WIP] ResourceList: onClick is called three times on keypress of Enter Mar 13, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 15, 2019 13:35 Inactive
@alex-page alex-page changed the title [WIP] ResourceList: onClick is called three times on keypress of Enter ResourceList: onClick is called three times on keypress of Enter Mar 15, 2019
@alex-page
Copy link
Member Author

@chloerice This should be good to go now :)

@alex-page alex-page self-assigned this Mar 15, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 15, 2019 13:44 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 20, 2019 13:33 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 20, 2019 13:43 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 20, 2019 13:51 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 20, 2019 14:02 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1188 March 20, 2019 14:09 Inactive
@alex-page alex-page changed the title ResourceList: onClick is called three times on keypress of Enter ResourceList onClick is called three times on keypress of Enter Mar 27, 2019
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.

Left a few small comments, with a squash this should be 👍 . 🎩 and everything looked 👌

AndrewMusgrave and others added 2 commits May 2, 2019 13:16
Co-Authored-By: alex-page <alex@alexpage.com.au>
Co-Authored-By: alex-page <alex@alexpage.com.au>
@danrosenthal
Copy link

What's the status on this PR @alex-page, does it need another round of review?

@alex-page
Copy link
Member Author

What's the status on this PR @alex-page, does it need another round of review?

I need to tweak the changes Andrew suggested. Should be ready tomorrow. Thanks for the bump!

@alex-page alex-page requested a review from AndrewMusgrave May 7, 2019 13:02
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.

LGTM 👍 You'll want to rebase to squash the commits/resolve conflicts, good luck 😄

@alex-page alex-page merged commit 34c1b9f into master May 8, 2019
@alex-page alex-page deleted the fix-resource-list-keypress-multi-click-fire branch May 8, 2019 13:28
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.Item] onClick is called three times on keypress of Enter

5 participants