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

fix(modal): fall back to modal id for dialog aria-labelledby attribute #1403

Merged
merged 1 commit into from
May 28, 2024

Conversation

kevinbuhmann
Copy link
Member

@kevinbuhmann kevinbuhmann commented May 28, 2024

PR Checklist

  • Tests for the changes have been added (for bug fixes / features)
  • [N/A] Docs have been added / updated (for bug fixes / features)
  • [N/A] If applicable, have a visual design approval

PR Type

Bugfix

What is the current behavior?

If you set [clrModalLabelledById] to an empty string, the aria-labelledby attribute of the modal dialog element is set to an empty value.

This behavior can be seen in the modal stories since the input is defaulted to an empty string. We can't default it to the modal id because that is not known until the component is instantiated.

Issue Number: CDE-2050

What is the new behavior?

If you set [clrModalLabelledById] to an empty string, the aria-labelledby attribute of the modal dialog element is set to the modal id.

Does this PR introduce a breaking change?

No.

Copy link
Contributor

github-actions bot commented May 28, 2024

👋 @kevinbuhmann,

  • 🙏 The Clarity team thanks you for opening a pull request
  • 🎉 The build for this PR has succeeded
  • 🔍 The PR is now ready for review
  • 🍿 In the meantime, view a preview of this PR
  • 🖐 You can always follow up here. If you're a VMware employee, you can also reach us on our internal Clarity Support space

Thank you,

🤖 Clarity Release Bot

@kevinbuhmann kevinbuhmann changed the title fix(modal): use modal id for dialog aria-labelledby attribute by default fix(modal): fall back to modal id for dialog aria-labelledby attribute May 28, 2024
@kevinbuhmann kevinbuhmann changed the title fix(modal): fall back to modal id for dialog aria-labelledby attribute fix(modal): fall back to modal id for dialog aria-labelledby attribute May 28, 2024
Copy link
Contributor

@Jinnie Jinnie left a comment

Choose a reason for hiding this comment

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

Good catch

@kevinbuhmann kevinbuhmann merged commit 6493dc3 into main May 28, 2024
7 checks passed
@kevinbuhmann kevinbuhmann deleted the kevin/modal-labelledby branch May 28, 2024 16:34
github-actions bot pushed a commit that referenced this pull request May 29, 2024
…ute (#1403)

## What is the current behavior?

If you set `[clrModalLabelledById]` to an empty string, the
`aria-labelledby` attribute of the modal dialog element is set to an
empty value.

This behavior can be seen in the modal stories since the input is
defaulted to an empty string. We can't default it to the modal id
because that is not known until the component is instantiated.

Issue Number: CDE-2050

## What is the new behavior?

If you set `[clrModalLabelledById]` to an empty string, the
`aria-labelledby` attribute of the modal dialog element is set to the
modal id.

(cherry picked from commit 6493dc3)
Jinnie pushed a commit that referenced this pull request May 29, 2024
…ute (#1403)

If you set `[clrModalLabelledById]` to an empty string, the
`aria-labelledby` attribute of the modal dialog element is set to an
empty value.

This behavior can be seen in the modal stories since the input is
defaulted to an empty string. We can't default it to the modal id
because that is not known until the component is instantiated.

Issue Number: CDE-2050

If you set `[clrModalLabelledById]` to an empty string, the
`aria-labelledby` attribute of the modal dialog element is set to the
modal id.

 (cherry picked from commit 6493dc3)
Jinnie pushed a commit that referenced this pull request May 29, 2024
…ute (backport to 16.x) (#1407)

Backport 6493dc3 from #1403. <br> ## PR
Checklist

- [x] Tests for the changes have been added (for bug fixes / features)
- [N/A] Docs have been added / updated (for bug fixes / features)
- [N/A] If applicable, have a visual design approval

## PR Type

Bugfix

## What is the current behavior?

If you set `[clrModalLabelledById]` to an empty string, the
`aria-labelledby` attribute of the modal dialog element is set to an
empty value.

This behavior can be seen in the modal stories since the input is
defaulted to an empty string. We can&#39;t default it to the modal id
because that is not known until the component is instantiated.

Issue Number: CDE-2050

## What is the new behavior?

If you set `[clrModalLabelledById]` to an empty string, the
`aria-labelledby` attribute of the modal dialog element is set to the
modal id.

## Does this PR introduce a breaking change?

No.

Co-authored-by: Kevin Buhmann <kbuhmann@vmware.com>
Copy link
Contributor

🎉 This PR is included in version 17.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link
Contributor

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed PRs after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants