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

[Accessibility] Button fixes #274

Merged
merged 6 commits into from
Oct 29, 2020
Merged

[Accessibility] Button fixes #274

merged 6 commits into from
Oct 29, 2020

Conversation

aleksik
Copy link
Contributor

@aleksik aleksik commented Oct 12, 2020

WIP - Requires https://helsinkisolutionoffice.atlassian.net/browse/HDS-449

Description

Accessibility fixes for the Button component:

  • Set button minimum width and height to 44px
  • Require iconLeft or iconRight for supplementary button
  • Changed documentation to mention that Supplementary buttons should not be used without an icon
  • Added a mention that HDS Buttons comply with WCAG Target Size guideline
  • Shortened the spacing between icon and label in Supplementary button to 4px

How Has This Been Tested?

Tested by running Storybook and documentation site locally.

@aleksik aleksik added the enhancement Request related to a current component/pattern/token/documentation. label Oct 12, 2020
@ronijaakkola
Copy link
Contributor

Since mobile navigation uses a supplementary button as a "sign out" button, we need to add an icon to it to comply with this new requirement.

The icon we should use is signout.

image

Copy link
Contributor

@niglu1 niglu1 left a comment

Choose a reason for hiding this comment

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

👍

@ronijaakkola
Copy link
Contributor

@aleksik Documentation was done in this branch. We could merge these together.
#262

@aleksik
Copy link
Contributor Author

aleksik commented Oct 15, 2020

@ronijaakkola I cherry-picked your documentation commit to this branch.

#262 can now be closed.

@ronijaakkola
Copy link
Contributor

@ronijaakkola I cherry-picked your documentation commit to this branch.

#262 can now be closed.

Thanks! I will check the documentation and update the examples if needed.

Copy link
Contributor

@niglu1 niglu1 left a comment

Choose a reason for hiding this comment

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

I noticed that the size and margins of the small button icons are wrong. According to the button design, the size should be the same as for the normal button. The margin between the label and the left/right icons should be 8px, now it's 12px, so it should be changed to use the var(--spacing-2-xs) token. Maybe that could be changed as part of this task as we're changing the small button appearance anyway?

/* left - small */
.hds-button--small .hds-icon {
  height: 1.125em;
  margin-left: var(--spacing-xs);
  width: 1.125em;
}

@aleksik aleksik force-pushed the accessibility/button-fixes branch from 93d0c71 to 5c090b2 Compare October 21, 2020 08:47
@aleksik aleksik requested a review from niglu1 October 21, 2020 08:47
@aleksik aleksik force-pushed the accessibility/button-fixes branch from 5c090b2 to bd292ea Compare October 28, 2020 15:32
@aleksik aleksik marked this pull request as ready for review October 28, 2020 15:32
@niglu1 niglu1 merged commit fd28a83 into master Oct 29, 2020
@niglu1 niglu1 deleted the accessibility/button-fixes branch October 29, 2020 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request related to a current component/pattern/token/documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants