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

Remove duplicate aria-label in CloseButton #1805

Merged
merged 2 commits into from
Feb 2, 2023

Conversation

paulcsmith
Copy link
Contributor

@paulcsmith paulcsmith commented Feb 2, 2023

Description

The CloseButton had an issue where using aria: { label: "foo" } would result in multiple aria-label attributes in the generated HTML.

This fixes the issue by using the aria helper method that will check for nested hash keys or a String aria-label

Integration

Does this change require any updates to code in production?

No. This should work as-is

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews

@changeset-bot
Copy link

changeset-bot bot commented Feb 2, 2023

🦋 Changeset detected

Latest commit: 0e801c5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

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

@@ -28,7 +28,7 @@ def initialize(type: DEFAULT_TYPE, **system_arguments)
"close-button",
system_arguments[:classes]
)
@system_arguments[:"aria-label"] ||= "Close"
@system_arguments[:"aria-label"] = aria("label", system_arguments) || "Close"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The aria method gets the attribute from either a String or a nested hash:

def aria(val, system_arguments)
system_arguments[:"aria-#{val}"] || system_arguments.dig(:aria, val.to_sym)
end

@paulcsmith paulcsmith marked this pull request as ready for review February 2, 2023 17:13
@paulcsmith paulcsmith requested review from a team and keithamus February 2, 2023 17:13
@paulcsmith paulcsmith changed the title More permissive aria-label Remove duplicate aria-label in CloseButton Feb 2, 2023
Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

Nice catch

@jonrohan jonrohan merged commit 238328a into primer:main Feb 2, 2023
@paulcsmith paulcsmith deleted the pcs/permissive-aria-label branch February 2, 2023 17:42
@primer-css primer-css mentioned this pull request Feb 2, 2023
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.

2 participants