Skip to content

Conversation

@dleroux
Copy link
Contributor

@dleroux dleroux commented Aug 15, 2019

WHY are these changes introduced?

Fixes Shortcut actions part of #792

Shortcut actions

  1. Smaller-breakpoint, persistent shortcut action buttons have no text equivalent for the ... icon. Screen reader users may not be able to tell what the button does.

  2. Controls that repeat for persistent shortcut actions for each list item do not have unique text. Non-visual users may have difficulty distinguishing which resource item they relate to as they tab through the controls and hear them read.

WHAT is this pull request doing?

For item 1 the wrong attribute was being added to the button so changing it to aria-label to accessibilityLabel makes the already translated label appear. However @dpersing you suggested a label like: "Actions for Mae Jemison". Since we don't have control over what people put in the list item in order to have that amount of detail we would need to add another prop to ResourceList which is already a bit bloated. Therefore I'm following the approach we did elsewhere: If there's is an accessibility label provided it will be "Actions for {accessibilityLabel}" if not it will just be the current translated text "Actions Dropdown" This won't actually make sense. For now, it just says "Actions Dropdown". If this is acceptable then we should keep this on our radar because I believe the ResourceList will be getting a bit of an overhaul soon.

For item 2 we do have that flexibility but it is up to the user to provide the action with the right label. I updated the example to show that.

How to 🎩

  1. Ensure that the labels for these 2 items are correct.
  2. Since there is a change to the documentation, yarn build-consumer polaris-styleguide to ensure the documentation renders properly.
Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page, ResourceList, Card, Avatar, TextStyle} from '../src';

export default function Playground() {
  return (
    <Page title="Playground">
      <ResourceListExample />
    </Page>
  );
}

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

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

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

  render() {
    const resourceName = {
      singular: 'customer',
      plural: 'customers',
    };
    const items = [
      {
        id: 341,
        url: 'customers/341',
        name: 'Mae Jemison',
        location: 'Decatur, USA',
        latestOrderUrl: 'orders/1456',
      },
      {
        id: 256,
        url: 'customers/256',
        name: 'Ellen Ochoa',
        location: 'Los Angeles, USA',
        latestOrderUrl: 'orders/1456',
      },
    ];
    return (
      <React.Fragment>
        <Card>
          <ResourceList
            resourceName={resourceName}
            items={items}
            renderItem={this.renderItem}
            selectedItems={this.state.selectedItems}
            onSelectionChange={this.handleSelectionChange}
            selectable
          />
        </Card>
      </React.Fragment>
    );
  }
}

🎩 checklist

@dleroux dleroux requested a review from dpersing August 15, 2019 13:22
@BPScott BPScott temporarily deployed to polaris-react-pr-1973 August 15, 2019 13:23 Inactive
@dleroux dleroux force-pushed the 792-resource-list-shortcut-actions branch from 693404e to da61f8e Compare August 15, 2019 13:29
@BPScott BPScott temporarily deployed to polaris-react-pr-1973 August 15, 2019 13:29 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1973 August 15, 2019 16:30 Inactive
@dleroux dleroux force-pushed the 792-resource-list-shortcut-actions branch from 6ac044d to e28a389 Compare August 15, 2019 16:47
@BPScott BPScott temporarily deployed to polaris-react-pr-1973 August 15, 2019 16:47 Inactive
@dleroux dleroux force-pushed the 792-resource-list-shortcut-actions branch from e28a389 to ab38314 Compare August 15, 2019 16:51
@BPScott BPScott temporarily deployed to polaris-react-pr-1973 August 15, 2019 16:51 Inactive
@dleroux dleroux force-pushed the 792-resource-list-shortcut-actions branch from ab38314 to 7e9d781 Compare August 15, 2019 17:00
@BPScott BPScott temporarily deployed to polaris-react-pr-1973 August 15, 2019 17:00 Inactive
@dleroux dleroux changed the title [WIP][ResourceList] Make item actions accessible [ResourceList] Make item actions accessible Aug 15, 2019
@dpersing
Copy link

Since we don't have control over what people put in the list item in order to have that amount of detail we would need to add another prop to ResourceList which is already a bit bloated.

What do you think about leveraging the name prop, like we do for the resource link, if one is provided, then falling back to a generic label if one isn't? We could update the documentation to include guidance around including a unique name for each item.

Copy link

@dpersing dpersing left a comment

Choose a reason for hiding this comment

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

This looks great for now. I will follow up with @AndrewMusgrave when he's back about standardizing using name or another way of creating better labels for all of the controls.

@dleroux
Copy link
Contributor Author

dleroux commented Aug 16, 2019

I'll leave this open depending on the timeline of the refactor. If it's too long let's just add name to the Item component.

@dleroux dleroux merged commit 6ba451d into master Aug 16, 2019
@dleroux dleroux deleted the 792-resource-list-shortcut-actions branch August 16, 2019 17:20
@dleroux dleroux temporarily deployed to production September 3, 2019 14:21 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.

3 participants