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

Live Region #3337

Merged
merged 9 commits into from
Sep 27, 2024
Merged

Live Region #3337

merged 9 commits into from
Sep 27, 2024

Conversation

AAwouters
Copy link
Contributor

Description

This PR adds the LiveRegion Service & Component. The live region component is invisible by default, but any messages pushed to the service are added to the component so screen readers can announce the message.

By itself this PR does very little but is intended as building block for issues such as #1271 and #3154.

Instructions for Reviewers

At the moment of this PR, the functionality of the live region is not used by any component so to test it a reviewer will have to do this manually.

  1. In config.dev.yml (or equivalent) add liveRegion: isVisible: true to make the live region visible at the bottom of the page.
  2. In any component with an interaction, add a message to the live service.
  3. The message should be visible in the live region at the bottom and should be announced by a screen reader

One option to quickly test the region is to add the following code to the body of the live-region.component.ts:

@HostListener('window:click')  
click() {  
  this.liveRegionService.addMessage('Click!');  
}

This will add the message "Click!" to the live region every time the reviewer clicks the mouse button.

CheckList

  • PRs should be smaller in size (ideally less than 1,000 lines of code, not including comments & tests)
  • PRs must pass ESLint validation using yarn lint
  • PRs must not introduce circular dependencies (verified via yarn check-circ-deps)
  • PRs must include TypeDoc comments for all new (or modified) public methods and classes. Large or complex private methods should also have TypeDoc.
  • PRs must pass all automated pecs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • User interface changes must align with Accessibility guidelines not applicable
  • PRs must use i18n (internationalization) keys instead of hardcoded English text, to allow for translations. not applicable
  • Details on how to test the PR must be provided. Reviewers must be aware of any steps they need to take to successfully test your fix or feature.
  • If a PR includes new libraries/dependencies (in package.json), then their software licenses must align with the DSpace BSD License based on the Licensing of Contributions documentation. not applicable
  • Basic technical documentation should be provided for any new features or configuration, either in the PR itself or in the DSpace Wiki documentation.
  • If a PR fixes an issue ticket, please link them together.

# Conflicts:
#	config/config.example.yml
#	src/app/shared/shared.module.ts
#	src/config/app-config.interface.ts
#	src/config/default-app-config.ts
#	src/environments/environment.test.ts
@AndreaBarbasso
Copy link
Contributor

I think the PR is good and very helpful in closing some other accessibility tasks. I'm eager for this to be merged!

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

@AAwouters : This looks great to me, but I'd ask that you add some basic TypeDocs to the new LiveRegion service & component. These TypeDocs/comments are useful for other developers to understand the purpose of these new classes.

Once that is added, I'll merge this immediately.

# Conflicts:
#	src/app/shared/live-region/live-region.component.ts
#	src/app/shared/live-region/live-region.service.spec.ts
@tdonohue tdonohue added port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Sep 27, 2024
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @AAwouters ! This looks good now. Because this is related to accessibility & other accessibility fixes will likely depend on it, I'm flagging this for backport to 8.x and 7.6.x. This should trigger an automated backport...but if one of those fail, we may need to manually backport.

(Sidenote: the Codecov coverage failure in this PR appears to be a false positive.)

@tdonohue tdonohue merged commit 052b6a9 into DSpace:main Sep 27, 2024
12 of 13 checks passed
@dspace-bot
Copy link
Contributor

Backport failed for dspace-7_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-7_x
git worktree add -d .worktree/backport-3337-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-3337-to-dspace-7_x
git switch --create backport-3337-to-dspace-7_x
git cherry-pick -x 83a44ba924fb20065f0bb62fc2a09a7aaec391bc e987c35450f2a03e73fe5a9951ec412fb9675e33 35d29c84258e1656ed17c53e6e7054c69b7878d1 c1fa52ee64e7c50f38c0e671aa13757bc7a4c024 751d689ff65b15fa4f8b2f898d43ecaa682e8085

@dspace-bot
Copy link
Contributor

Backport failed for dspace-8_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-8_x
git worktree add -d .worktree/backport-3337-to-dspace-8_x origin/dspace-8_x
cd .worktree/backport-3337-to-dspace-8_x
git switch --create backport-3337-to-dspace-8_x
git cherry-pick -x 83a44ba924fb20065f0bb62fc2a09a7aaec391bc e987c35450f2a03e73fe5a9951ec412fb9675e33 35d29c84258e1656ed17c53e6e7054c69b7878d1 c1fa52ee64e7c50f38c0e671aa13757bc7a4c024 751d689ff65b15fa4f8b2f898d43ecaa682e8085

@tdonohue
Copy link
Member

@AAwouters : Could you create a backport of this PR for the dspace-8_x and dspace-7_x branches? It appears the backports could not be done automatically, so we'll need to manually create backport PRs. (see errors above from dspace-bot)

@tdonohue tdonohue removed port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Sep 30, 2024
@tdonohue
Copy link
Member

Ported to 7.6.x in #3371 and 8.x in #3370

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

5 participants