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

(release-4.0.0) hds-2522: clean up announcements #1427

Merged
merged 4 commits into from
Nov 20, 2024

Conversation

NikoHelle
Copy link
Contributor

@NikoHelle NikoHelle commented Nov 18, 2024

Description

Code cleaning for the announcements. Also the banner's aria-live element was always removed, never announced. Made it visible to screen readers only.

Related Issue

Closes HDS-2522

Motivation and Context

How Has This Been Tested?

Added e2e tests

Demos:

Links to demos are in the comments

Screenshots (if appropriate):

Add to changelog

  • no need

Sometimes the element need to stay. Not sure when exactly, but did not change it in this PR.
The element must be visually hidden.

It is outside of added style element's scope, so have to hide via style attribute.
Copy link

Preview found from hds-demo docs/preview_1427

Demos

Docs
Core Storybook
React Storybook

Copy link

Test Results

  1 files   64 suites   11m 49s ⏱️
 59 tests  59 ✅ 0 💤 0 ❌
118 runs  118 ✅ 0 💤 0 ❌

Results for commit c788dd9.

Copy link
Contributor

@mrTuomoK mrTuomoK left a comment

Choose a reason for hiding this comment

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

Nothing else to say but the commented code, good work!

@mrTuomoK mrTuomoK self-requested a review November 19, 2024 14:00
Copy link
Contributor

@mrTuomoK mrTuomoK left a comment

Choose a reason for hiding this comment

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

All good now, good job!

@NikoHelle NikoHelle merged commit 461d873 into release-4.0.0 Nov 20, 2024
8 checks passed
@NikoHelle NikoHelle deleted the hds-2522-announcement-removal branch November 20, 2024 06:53
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