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

[A11y][Filters] position hidden proxy container offscreen to prevent focus #4073

Merged
merged 3 commits into from
Mar 24, 2021

Conversation

lhoffbeck
Copy link
Contributor

WHY are these changes introduced?

Fixes @shopify/web issue #37890

Currently, if you tab into a <Filters> component from a component that relies on the focus.ts utility to determine the nextFocuseableNode (e.g. Popover, Modal, etc), the focus state is lost and returned to the window.

This happens because the ConnectedFilterControl subcomponent on Filters has a hidden ProxyButtonContainer that focus.ts picks as the next focusable node programmatically, although you can't tab to it normally.

(similar approach on an IndexTable fix here)

👀 before 👀

before.mp4

👀 after 👀

after.mp4

WHAT is this pull request doing?

This PR positions the hidden button container out of the viewport so the focus utility skips over it.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

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

import {
  Avatar,
  Card,
  ChoiceList,
  Filters,
  Page,
  ResourceList,
  Tabs,
  TextField,
  TextStyle,
} from '../src';

export function Playground() {
  const [selected] = useState(0);

  const tabs = [
    {
      id: 'all-customers-1',
      content: 'All',
      accessibilityLabel: 'All customers',
      panelID: 'all-customers-content-1',
    },
    {
      id: 'accepts-marketing-1',
      content: 'Accepts marketing',
      panelID: 'accepts-marketing-content-1',
    },
    {
      id: 'repeat-customers-1',
      content: 'Repeat customers',
      panelID: 'repeat-customers-content-1',
    },
    {
      id: 'prospects-1',
      content: 'Prospects',
      panelID: 'prospects-content-1',
    },
    {
      id: 'all-customers-2',
      content: 'All',
      accessibilityLabel: 'All customers',
      panelID: 'all-customers-content-1',
    },
    {
      id: 'accepts-marketing-2',
      content: 'Accepts marketing',
      panelID: 'accepts-marketing-content-1',
    },
    {
      id: 'repeat-customers-2',
      content: 'Repeat customers',
      panelID: 'repeat-customers-content-1',
    },
    {
      id: 'prospects-2',
      content: 'Prospects',
      panelID: 'prospects-content-1',
    },
    {
      id: 'all-customers-13',
      content: 'All',
      accessibilityLabel: 'All customers',
      panelID: 'all-customers-content-1',
    },
    {
      id: 'accepts-marketing-13',
      content: 'Accepts marketing',
      panelID: 'accepts-marketing-content-1',
    },
    {
      id: 'repeat-customers-13',
      content: 'Repeat customers',
      panelID: 'repeat-customers-content-1',
    },
    {
      id: 'prospects-13',
      content: 'Prospects',
      panelID: 'prospects-content-1',
    },
  ];

  return (
    <Page title="Playground">
      <div style={{height: '568px'}}>
        <Card>
          <Tabs tabs={tabs} selected={selected} />
          <ResourceListFiltersExample />
        </Card>
      </div>
    </Page>
  );
}

function ResourceListFiltersExample() {
  const filters = [
    {
      key: 'taggedWith',
      label: 'Tagged with',
      filter: (
        <TextField
          label="Tagged with"
          value=""
          onChange={() => {}}
          labelHidden
        />
      ),
      shortcut: true,
    },
    {
      key: 'accountStatus',
      label: 'Account status',
      filter: (
        <ChoiceList
          title="Account status"
          titleHidden
          choices={[
            {label: 'Enabled', value: 'enabled'},
            {label: 'Not invited', value: 'not invited'},
            {label: 'Invited', value: 'invited'},
            {label: 'Declined', value: 'declined'},
          ]}
          selected={[]}
          onChange={() => {}}
          allowMultiple
        />
      ),
      shortcut: true,
    },
  ];

  return (
    <ResourceList
      resourceName={{singular: 'customer', plural: 'customers'}}
      filterControl={
        <Filters
          queryValue=""
          filters={filters}
          appliedFilters={[]}
          onQueryChange={() => {}}
          onQueryClear={() => {}}
          onClearAll={() => {}}
        />
      }
      items={[
        {
          id: 341,
          url: 'customers/341',
          name: 'Mae Jemison',
          location: 'Decatur, USA',
        },
        {
          id: 256,
          url: 'customers/256',
          name: 'Ellen Ochoa',
          location: 'Los Angeles, USA',
        },
      ]}
      renderItem={(item) => {
        const {id, url, name, location} = item;
        const media = <Avatar customer size="medium" name={name} />;

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

🎩 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

@lhoffbeck lhoffbeck requested a review from a team March 22, 2021 20:45
@github-actions
Copy link
Contributor

github-actions bot commented Mar 22, 2021

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

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

Files potentially affected (total: 0)

🎨 src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss (total: 2)

Files potentially affected (total: 2)

@lhoffbeck lhoffbeck changed the title [Filters] position hidden proxy container offscreen to prevent focus [A11y][Filters] position hidden proxy container offscreen to prevent focus Mar 23, 2021
Copy link
Contributor

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

I'm not a fan of magic numbers as a solution but this seems tried and tested in web 👍

Thanks!

UNRELEASED.md Outdated Show resolved Hide resolved
Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com>
@lhoffbeck lhoffbeck merged commit 278f5ce into main Mar 24, 2021
@lhoffbeck lhoffbeck deleted the fix-connected-filter-focus-issue branch March 24, 2021 12:42
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.

None yet

2 participants