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: customise and move location of Notice component More Information button #3947

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

augustlindemer
Copy link
Collaborator

This PR ensures that the Notice component has a defined aria-label for its More information button and displays this button below its content block (if defined).

It replaces the use of CardHeader in the Notice component with a custom More information layout using a static aria-label (copying the approach used for the Content component).

info={props.info}
policyRef={props.policyRef}
howMeasured={props.howMeasured}
/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Notice component uses CardHeader for its More Information button. Because we don't want a title above the content block (the notice), it does so without passing a title prop. Because CardHeader uses the title prop to customise the aria-label of the More information button, the Notice component returns undefined in this aria-label.

Copy link
Member

Choose a reason for hiding this comment

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

Great spot & accessibility fix 🌟

Copy link

github-actions bot commented Nov 13, 2024

Removed vultr server and associated DNS entries

Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Works exactly as expected - nicely implemented 🙌

React tests failed on first run on an un-related "Send" component test that tends to be a bit flakely, a simple "re-run failed jobs" did the trick. Happy for this to be "squashed & merged" whenever ✅

info={props.info}
policyRef={props.policyRef}
howMeasured={props.howMeasured}
/>
Copy link
Member

Choose a reason for hiding this comment

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

Great spot & accessibility fix 🌟

@augustlindemer augustlindemer merged commit 0fa25c1 into main Nov 13, 2024
12 checks passed
@augustlindemer augustlindemer deleted the augustlindemer-notice-component-more-info branch November 13, 2024 15:06
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