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

[SettingToggle]: Implement accessibility role and attributes #5470

Merged
merged 9 commits into from
May 17, 2022

Conversation

joelzwarrington
Copy link
Contributor

@joelzwarrington joelzwarrington commented Apr 7, 2022

WHY are these changes introduced?

Fixes #5462

WHAT is this pull request doing?

  • Allow aria-checked attribute to be passed to <Button />
  • Set role on SettingAction button to switch
  • Set aria-checked attributes on Setting Action based on enabled value
  • Labelling the switch with the content passed in as children
  • Reflected changes in the SettingToggle accessibility documentation
Screenshot showing which sub-component becomes the label & switch Screenshot showing which sub-component becomes the label & switch

There should be no visual change in this PR to the SettingToggle component.

SettingToggle.-.Confirming.aria.checked.interaction.mov

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

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

export function Playground() {
  const [active, setActive] = useState(false);

  const handleToggle = useCallback(() => setActive((active) => !active), []);

  const contentStatus = active ? 'Deactivate' : 'Activate';
  const textStatus = active ? 'activated' : 'deactivated';

  return (
    <Page title="Playground">
      <SettingToggle
        action={{
          content: contentStatus,
          onAction: handleToggle,
        }}
        enabled={active}
      >
        This setting is <TextStyle variation="strong">{textStatus}</TextStyle>.
      </SettingToggle>
    </Page>
  );
}

🎩 checklist

@ghost
Copy link

ghost commented Apr 7, 2022

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

size-limit report 📦

Path Size
cjs 150.79 KB (+0.06% 🔺)
esm 86.92 KB (+0.08% 🔺)
esnext 136.79 KB (+0.06% 🔺)
css 37.22 KB (0%)

@joelzwarrington
Copy link
Contributor Author

@chloerice / @alex-page do you know who I should ping to get this reviewed?

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.

Code looks good. Thanks @joelzwarrington

polaris-react/UNRELEASED.md Outdated Show resolved Hide resolved
polaris-react/src/types.ts Outdated Show resolved Hide resolved
@joelzwarrington joelzwarrington force-pushed the 5462/setting-toggle-accessibility branch from 874f872 to 72b1785 Compare May 17, 2022 16:05
@changeset-bot
Copy link

changeset-bot bot commented May 17, 2022

⚠️ No Changeset found

Latest commit: 48bc383

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@joelzwarrington joelzwarrington merged commit 45dd1b2 into main May 17, 2022
@joelzwarrington joelzwarrington deleted the 5462/setting-toggle-accessibility branch May 17, 2022 16:44
@ghost
Copy link

ghost commented May 17, 2022

🎉 Thanks for your contribution to Polaris!

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.

Setting Toggle: improve accessibility by using switch role and related attributes
3 participants