[ResourceList] Add alternateTool prop

[dleroux]

WHY are these changes introduced?

This PR is put in place to achieve this without needing to fork the list.

Currently the only item that can go in the top right of the ResourceList is the ResourceList.Sort. There are instances where that area could be used for different content. In this particular case a popover of edit actions.

WHAT is this pull request doing?

Adding a prop of customTool that can take any ReactNode
Note that this customTool is added instead of the sort, because the current design does not lend itself well to having more than 1 popover on that row.

Before completing this PR with tests I'm looking for feedback on

1. Do we want to do this? The reason I am for it is because this looks like an easy win to add some level of flexibility to the list. Worth preventing another fork imo.

2. How do we feel about alternateTool as a prop name.

3. Options of this being rendered as oppose to Sort instead of giving the option of rendering both the Sort and the Custom Tool.

How to 🎩

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

/* eslint-disable react/jsx-no-bind */

import * as React from 'react';
import {
  Page,
  ResourceList,
  Card,
  FilterType,
  AppProvider,
  Layout,
  Sticky,
  Button,
  Popover,
  OptionList,
} from '@shopify/polaris';
import {autobind} from '@shopify/javascript-utilities/decorators';

interface State {
  searchValue?: string;
  selectedItems: string[] | 'All';
  sortValue?: string;
  openPopover: boolean;
}

export default class Playground extends React.Component<never, State> {
  state: State = {
    selectedItems: [],
    openPopover: false,
  };

  render() {
    const {openPopover} = this.state;

    const resourceName = {
      singular: 'Product',
      plural: 'Products',
    };

    const options = [
      {label: 'Open bulk editor', value: ''},
      {label: 'Edit prices', value: ''},
      {label: 'Edit quantities', value: ''},
      {label: 'Edit option values', value: ''},
      {label: 'Edit SKUs', value: ''},
      {label: 'Edit barcodes', value: ''},
    ];

    const alternateToolMarkup = (
      <Popover
        active={openPopover}
        activator={
          <Button disclosure onClick={() => this.setState({openPopover: true})}>
            Edit Variants
          </Button>
        }
        onClose={() => this.setState({openPopover: false})}
      >
        <Popover.Pane>
          <OptionList options={options} selected={[]} onChange={() => null} />
        </Popover.Pane>
      </Popover>
    );

    return (
      <AppProvider>
        <Page title="Playground">
          <Layout>
            <Layout.Section>
              <Card>
                <ResourceList
                  items={items}
                  renderItem={handleRenderItem}
                  resourceName={resourceName}
                  alternateTool={alternateToolMarkup}
                  selectedItems={this.state.selectedItems}
                  onSelectionChange={this.handleSelectionChange}
                  promotedBulkActions={[
                    {
                      content: 'Really long text on button 1',
                      onAction: this.bulkActionOne,
                    },
                    {
                      content: 'long text button 2',
                      disabled: true,
                      url: 'http://www.google.com',
                    },
                  ]}
                  bulkActions={[
                    {
                      content: 'button 3',
                      onAction: this.bulkActionThree,
                    },
                    {
                      content: 'button 4',
                      onAction: this.bulkActionFour,
                    },
                    {
                      content: 'button 5',
                      onAction: this.bulkActionFive,
                      disabled: true,
                    },
                  ]}
                  sortValue={this.state.sortValue}
                  sortOptions={mockSortOptions}
                  onSortChange={this.handleSortChange}
                />
              </Card>
            </Layout.Section>
            <Layout.Section secondary>
              <Sticky offset disableWhenStacked>
                <div style={{paddingBottom: '20px'}}>
                  <Card title="Tags" sectioned>
                    <p>Add tags to your order.</p>
                  </Card>
                </div>
              </Sticky>
              <Card title="Tags" sectioned>
                <p>Add tags to your order.</p>
              </Card>
              <Card title="Tags" sectioned>
                <p>Add tags to your order.</p>
              </Card>
            </Layout.Section>
          </Layout>
        </Page>
      </AppProvider>
    );
  }

  @autobind
  private handleSelectionChange(selectedItems: string[]) {
    this.setState({selectedItems});
  }

  @autobind
  private handleSearchChange(searchValue: string) {
    this.setState({searchValue});
  }

  @autobind
  private handleSortChange(sortValue: string) {
    this.setState({sortValue});
  }

  @autobind
  private bulkActionOne() {
    console.log('Clicked on bulk action one.');
  }

  @autobind
  private bulkActionThree() {
    console.log('Clicked on bulk action three.');
  }

  @autobind
  private bulkActionFour() {
    console.log('Clicked on bulk action four.');
  }

  @autobind
  private bulkActionFive() {
    console.log('Clicked on bulk action five.');
  }
}

function handleRenderItem(item: any, id: any) {
  return (
    <ResourceList.Item
      id={id}
      url={item.url}
      shortcutActions={item.actions}
      accessibilityLabel={`View details for ${item.title}`}
    >
      <div>Item {id}</div>
      <div>{item.title}</div>
    </ResourceList.Item>
  );
}

const items: any[] = [
  {
    onClick: true,
    url: 'https://www.google.com',
    actions: [{content: 'View listing', url: 'http://www.facebook.com'}],
    title: 'Has url, onClick, and actions',
  },
  {
    onClick: false,
    url: 'https://www.google.com',
    actions: [{content: 'View listing', url: 'http://www.facebook.com'}],
    title: 'Has url, and actions',
  },
  {
    onClick: true,
    actions: [{content: 'View listing', url: 'http://www.facebook.com'}],
    title: 'Has onClick, and actions',
  },
  {
    onClick: false,
    url: 'https://www.google.com',
    title: 'Has url',
  },
  {
    onClick: true,
    title: 'Has onClick',
  },
  {
    onClick: true,
    url: 'https://www.google.com',
    actions: [{content: 'View listing', url: 'http://www.facebook.com'}],
    title: 'Has url, onClick, and actions',
  },
];

const mockSortOptions = [
  'Product title (A-Z)',
  {
    value: 'PRODUCT_TITLE_DESC',
    label: 'Product title (Z-A)',
  },
  {
    value: 'EXTRA',
    label: 'Disabled Option',
    disabled: true,
  },
];

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[danrosenthal]

Before I get into your questions, my first reaction is this PR desperately needs some test coverage. But you were probably planning on following up with that. So here are my answers to your questions:

Do we want to do this? The reason I am for it is because this looks like an easy win to add some level of flexibility to the list. Worth preventing another fork imo.

I think this rationale makes sense. The tools need a slot to make this component more flexible for other use-cases.

How do we feel about customTool as a prop name.

I'm not crazy about using the word custom in a property name, though I am comfortable with the use of tool, as I have always thought of the header elements as tools for modifying the list. But if we can't come up with an alternative to custom, I suppose I can live with it.

Options of this being rendered as oppose to Sort instead of giving the option of rendering both the Sort and the Custom Tool.

I don't think that using this component should have any side effects. It should be up to the consumer whether sorting gets rendered, or it should follow the default behavior of the component. If this replaces the sort component visually, it might make sense just to allow sort to take a ReactNode, instead of creating a new prop.

[sarahill]

Do we want to do this? The reason I am for it is because this looks like an easy win to add some level of flexibility to the list. Worth preventing another fork imo.

I think this makes sense and gives a level of flexibility for the different teams.

How do we feel about customTool as a prop name.

I agree with @danrosenthal on this. Not crazy about using custom as part of the prop name. ListAction or ListTool?

Options of this being rendered as oppose to Sort instead of giving the option of rendering both the Sort and the Custom Tool.

I think having the option to render both makes sense. One thing to think about is how this looks on smaller screens.

[dleroux]

Yes thats the thing on small screen there really isn’t much room, it breaks the layout really quickly. On slack we discussed using ‘alternateTool’ which would make it more clear that this would replace the sort.

[sarahill]

That makes sense. I was getting hung up on the additional prop. Re-reading @danrosenthal 's comment makes more sense to me now.

If this replaces the sort component visually, it might make sense just to allow sort to take a ReactNode, instead of creating a new prop.

[dleroux]

This PR has been updated to use alternateTool. Because there isn't much room within our current breakpoint to add more than 1 tool in that corner using alternateTool makes it a bit clearer that there will be a side affect. The more appropriate prop name would have been tool but it wouldn't have been clear that this would replace the sort. In subsequent versions of the list we might consider deprecating the sorting props and the additionalTool prop for a single prop.

For now this is complete with tests, and a style-guide example.

[danrosenthal]

Looks good. Might be worth beefing up the rationale a bit, otherwise I think this is good to go. Nice test coverage!

[danrosenthal]

Might be worth adding some rationale on why they would want to do this.

[AndrewMusgrave]

Should we use a regular id here? 😄

[dleroux]

🙈 done.