Skip to content

Minor accessibility fixes#3251

Merged
kshepherd merged 4 commits intoDSpace:mainfrom
alexandrevryghem:w2p-110615_minor-accessibibity-fixes_contribute-main
Aug 29, 2024
Merged

Minor accessibility fixes#3251
kshepherd merged 4 commits intoDSpace:mainfrom
alexandrevryghem:w2p-110615_minor-accessibibity-fixes_contribute-main

Conversation

@alexandrevryghem
Copy link
Copy Markdown
Member

Description

These are minor fixes that were required in order to fix some issues regarding accessibility on the item page & the browse controlled vocabulary page.

Instructions for Reviewers

In order to detect these issues we used Silktide

List of changes in this PR:

  • Fixed issues where the search tabs on the item pages didn't have the correct roles (only an issue when multiple tabs are available)
  • Removed id browseDropdown from ExpandableNavbarSectionComponent, since it's not used anywhere an that component is also not meant to be browse specific. This caused a duplicate id issue when you used 2 expandable secitons in your navbar
  • Fixed missing label for the search input field on the browse by controlled vocabulary pages

Checklist

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

…licate ID issue by removing the unused #browseDropdown ID
…ute-7.2' into w2p-110615_minor-accessibibity-fixes_contribute-7.6

# Conflicts:
#	src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
…to w2p-110615_minor-accessibibity-fixes_contribute-main

# Conflicts:
#	src/app/item-page/simple/related-entities/tabbed-related-entities-search/tabbed-related-entities-search.component.html
#	src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html
#	src/app/shared/form/vocabulary-treeview/vocabulary-treeview.component.html
@alexandrevryghem alexandrevryghem added accessibility affects: 8.x Issue impacts 8.x releases affects: 7.x Issue impacts 7.x releases labels Aug 13, 2024
@alexandrevryghem alexandrevryghem self-assigned this Aug 13, 2024
@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 1 APPROVAL pull request only requires a single approval to merge labels Aug 21, 2024
@kshepherd kshepherd self-requested a review August 29, 2024 12:59
Copy link
Copy Markdown
Member

@kshepherd kshepherd left a comment

Choose a reason for hiding this comment

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

+1 thanks @alexandrevryghem , changes look good and i have tested them locally

@kshepherd kshepherd merged commit 332be2b into DSpace:main Aug 29, 2024
@dspace-bot
Copy link
Copy Markdown
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-3251-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-3251-to-dspace-7_x
git switch --create backport-3251-to-dspace-7_x
git cherry-pick -x a0515c412140184c23cface4febb5d48f0d430eb 54d99a84c9eb59335f9e62b3b043f86ccd1f80d3

@dspace-bot
Copy link
Copy Markdown
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-3251-to-dspace-8_x origin/dspace-8_x
cd .worktree/backport-3251-to-dspace-8_x
git switch --create backport-3251-to-dspace-8_x
git cherry-pick -x a0515c412140184c23cface4febb5d48f0d430eb 54d99a84c9eb59335f9e62b3b043f86ccd1f80d3

@kshepherd
Copy link
Copy Markdown
Member

@alexandrevryghem our auto-backports didn't work due to conflicts, would it be much trouble for you to push some hand-made PRs for the 7_x and 8_x branches?

@alexandrevryghem
Copy link
Copy Markdown
Member Author

@kshepherd: No this won't take much time, I created the code on the dspace-7.6 tag, so this should be fairly simple

@tdonohue tdonohue added this to the 9.0 milestone Aug 29, 2024
@tdonohue tdonohue changed the title Minor accessibibity fixes Minor accessibility fixes Aug 29, 2024
@alexandrevryghem alexandrevryghem deleted the w2p-110615_minor-accessibibity-fixes_contribute-main branch August 30, 2024 22:06
@alexandrevryghem alexandrevryghem 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 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge accessibility affects: 7.x Issue impacts 7.x releases affects: 8.x Issue impacts 8.x releases

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants