Issue 410 accessibility improvements #416

Dbreck-TAMU · 2025-01-22T17:30:07Z

Description

Resolves #410

…tio. The https://webaim.org/resources/contrastchecker/ is used to determine valid color contrast ratios.

…ements-color_contrast [Issue 410] Accessibility improvements color contrast.

…ements

…AMULib/scholars-angular into sprint9-410-accessibility-improvements

…tio. This handles additional areas missed by this commit 59c5735. The `::deep` is necessary for the dialog because the ngb-pagination is in a different scope. The global link colors use too light of a blue according to the scanners. Change the color to a darker color. The hover link is made darker to preserve the behavior of being darker on hover.

…ements-color_contrast-2 410: Manually override pagination colors to enforce color contrast ratio.

kaladay · 2025-01-24T20:56:24Z

src/app/+dashboard/about/about.component.html

+    <li><a href="https://scholars.library.tamu.edu/pdf/Scholars%20Admin%20Info%20Sheet.pdf" target="_blank" rel="noopener noreferrer">Benefits for Administrators</a></li>
+    <li><a href="https://scholars.library.tamu.edu/pdf/Scholars%20Faculty%20Info%20Sheet.pdf" target="_blank" rel="noopener noreferrer">Benefits for Faculty</a></li>
+    <li><a href="https://scholars.library.tamu.edu/pdf/infoSheet_ScholarsVersion.pdf" target="_blank" rel="noopener noreferrer">Enhanced Features for Scholars@TAMU (Updates of September 2019)</a></li>


I don't see the reason for these changes where target=... and rel=... are added.
I am not seeing a request from this on the form.
Are these added due to a WebAIM Wave error?

kaladay · 2025-01-24T20:57:24Z

src/app/+dashboard/about/about.component.html

+    <li><i class='fa fa-user mr-1'></i><a href="display/nf489b17d" aria-label="Dr. Bruce Herbert Profile">Dr. Bruce Herbert</a>, Director, Office of Scholarly Communications</li>
+    <li><i class='fa fa-user mr-1'></i><a href="display/nbef1f2f3" aria-label="Dr. Dong Joon Lee Profile">Dr. Dong Joon Lee</a>, Researcher Information Systems Librarian, Office of Scholarly Communications</li>
+    <li><i class='fa fa-user mr-1'></i><a href="display/n31ebd4a6" aria-label="Ethel Mejia Profile">Ethel Mejia</a>, Data Analyst, Office of Scholarly Communications</li>
+    <li><i class='fa fa-user mr-1'></i><a href="display/n3e0f7747" aria-label="Doug Hahn Profile">Doug Hahn</a>, Senior IT Manager, Applications, Digital Initiatives</li>


These aria-label are redundant. The text is already descriptive.
Please remove aria-label uses in this case.

kaladay · 2025-01-24T21:02:40Z

src/app/+dashboard/home/home.component.html

-<div class="home-main" *ngIf="home | async; let home">
-  <ngb-carousel *ngIf="showCarousel(home)">
+<div class="home-main" *ngIf="home | async; let home" aria-live="polite">
+  <ngb-carousel *ngIf="showCarousel(home)" aria-label="Main carousel showcasing heroes">


The use of aria-label here is unclear to me.
If the WebAIM Wave reports this as needed, then that is fine.
Otherwise, it seems out of place to add a label here.

If you think we should use this here, then we might better be served by using aria-labelledby and point it to the text using DASHBOARD.HOME.LATEST_PUBLICATIONS_TEXT.
(I think that text is on line 39 of home.component.html.)

A unique id attribute would need to be added to the tag using DASHBOARD.HOME.LATEST_PUBLICATIONS_TEXT in order to use the aria-labelledby.

kaladay · 2025-01-24T21:03:47Z

src/app/+dashboard/home/home.component.html

 <div class="home-recent mt-2 mb-2" *ngIf="home | async; let home">
  <div class="container">
    <div class="row">
      <div class="col-md-2 my-auto text-center">
        <span class="gallery-info">{{ 'DASHBOARD.HOME.LATEST_PUBLICATIONS_TEXT' | translate:{organization: (organization|async)} }}</span>
      </div>
      <div class="col-md-8 my-auto text-center">
-        <scholars-recent-carousel></scholars-recent-carousel>
+        <scholars-recent-carousel aria-label="Recent publications carousel"></scholars-recent-carousel>


This has the same situation as above regarding aria-labelledby.
In this case, you might want to add an id attribute to the tag using DASHBOARD.HOME.LATEST_PUBLICATIONS_TEXT.

kaladay · 2025-01-24T21:07:11Z

src/app/+dashboard/home/home.component.html

+      <scholars-stats-box label="People" classifier="Person" aria-label="People statistics"></scholars-stats-box>
+      <scholars-stats-box label="Organizations" classifier="Organization" aria-label="Organizations statistics"></scholars-stats-box>
+      <scholars-stats-box label="Research" classifier="Document" aria-label="Research statistics"></scholars-stats-box>


These are fine for now because there is no formal label.

However, with label="People" and label being defined inside the resulting expanded markup has people in it.
This can be changed to have an id and then given a aria-labelledby association.
This would avoid repeated the aria-label.

src/app/+directory/directory.component.html

src/app/+display/section/section.component.html

Co-authored-by: Kevin Day <[email protected]>

kaladay

Given the time frame that we have, I suggest that you back off on a lot of the id attribute changes due to a lot of problems with possible conflicts.

There is a lot of potential for non-distinct id name conflicts which itself is an accessibility problem.

kaladay · 2025-01-24T21:36:09Z

src/app/+display/tab/tab.component.html

+<div *ngIf="individual | async; let individual" class="tab-content" role="tabpanel" aria-live="polite">
+  <div *ngIf="tab | async; let tab" class="tab-pane active" [attr.aria-labelledby]="tab.id + '-tab'" role="tabpanel">
+    <div class="ml-3 mr-3 mt-3 card" *ngFor="let section of getSectionsToShow(tab, individual)">
+      <scholars-section [section]="section" [display]="display" [individual]="individual" aria-labelledby="section.id + '-section'"></scholars-section>


This one makes more sense.
You are using teh secion.id and concatting the -section.
This should be more distinct.
The other generic named id tags out there for generic components have me concerned.

Having said that, I am getting a lot of:

Broken ARIA reference
An aria-labelledby or aria-describedby reference exists, but the target for the reference does not exist.

Involving:

<scholars-section _ngcontent-ng-c1691634590="" aria-labelledby="section.id + '-section'" _nghost-ng-c2676255389="" ng-reflect-section="[object Object]" ng-reflect-display="Documents" ng-reflect-individual="[object Object]">

kaladay · 2025-01-24T21:37:25Z

src/app/+visualization/co-author-network/co-author-network.component.html

@@ -29,7 +29,7 @@
        <div class="card">
          <div class="card-header">{{ 'VISUALIZATION.NETWORK.COAUTHOR.OBJECTS_PER_YEAR' | translate }}</div>
          <div class="card-body">
-            <table class="table table-sm">
+            <table class="table table-sm" aria-labelledby="objectsPerYearTable">


I don't see a id tag anywhere with objectsPerYearTable.
Is this generated?

kaladay · 2025-01-24T21:37:46Z

src/app/+visualization/co-author-network/co-author-network.component.html

@@ -50,7 +50,7 @@
        <div class="card">
          <div class="card-header">{{ 'VISUALIZATION.NETWORK.COAUTHOR.COPARTICIPANT' | translate }}</div>
          <div class="card-body">
-            <table class="table table-sm">
+            <table class="table table-sm" aria-labelledby="coparticipantTable">


The same as the above comment about objectsPerYearTable.

In fact, it looks like all of the table cases are doing something like that.

kaladay · 2025-01-24T21:43:18Z

src/app/shared/alert/alert.component.html

-    <div [ngClass]="{'container': isMain()}"><span [innerHTML]="alert.message"></span></div>
+  <ngb-alert [type]="alert.type" [dismissible]="alert.dismissible" (closed)="onAlertClosed(alert)" [ngClass]="{'mt-0 mb-0 rounded-0 w-100': isMain()}" role="alert" aria-live="assertive" [attr.aria-label]="alert.type === 'danger' ? 'Error alert' : alert.type === 'warning' ? 'Warning alert' : 'Information alert'">
+    <div [ngClass]="{'container': isMain()}" role="alertdialog" aria-describedby="alert-message">
+      <span id="alert-message" [innerHTML]="alert.message"></span>


This must change.
The alert-message is not distinct and with the *ngFor above we will have non-distinct id attributes.

Also, the aria-describedby I would think should not be needed for when the content is nested inside.

kaladay · 2025-01-24T21:44:01Z

src/app/shared/dialog/dialog.component.html

  <div class="modal-header">
    <span class="h4 modal-title" id="modal-basic-title">{{dialog.title | async}}</span>
    <button *ngIf="dialog.close" type="button" class="close" aria-label="Close" (click)="dialog.close.action()" [disabled]="dialog.close.disabled() | async">
      <span aria-hidden="true">&times;</span>
    </button>
  </div>
-  <div class="modal-body">
-    <scholars-alert [location]="location"></scholars-alert>
+  <div class="modal-body" id="modal-body">


This id attribute has a chance of being non-distinct if there are emultiple dialogs on a single page.

src/app/+display/section/section.component.html

kaladay · 2025-01-24T22:10:24Z

src/app/+display/subsection/subsection.component.html

@@ -2,7 +2,7 @@
  <div *ngIf="getResources() | async as resources">
    <scholars-pagination *ngIf="resources.length > 0" [page]="page" [pageSizeOptions]="pageSizeOptions" [queryPrefix]="subsection.name">
      <div class="mb-3">
-        <h5 class="font-weight-bold">{{subsection.name}}<span class="badge badge-light ml-2">{{resources.length}}</span></h5>
+        <h5 id="pagination-label" class="font-weight-bold">{{subsection.name}}<span class="badge badge-light ml-2" aria-live="polite">{{resources.length}}</span></h5>


The same as previous statements about potential id attribute distinctness.

kaladay · 2025-01-24T22:12:59Z

src/app/+display/tab/tab.component.html

@@ -1,9 +1,7 @@
 <div *ngIf="individual | async; let individual" class="tab-content">
  <div *ngIf="tab | async; let tab" class="tab-pane active">


Add back the ; else no_content.

kaladay · 2025-01-24T22:16:50Z

src/app/+visualization/co-investigator-network/co-investigator-network.component.html

@@ -29,7 +29,7 @@
        <div class="card">
          <div class="card-header">{{ 'VISUALIZATION.NETWORK.COINVESTIGATOR.OBJECTS_PER_YEAR' | translate }}</div>
          <div class="card-body">
-            <table class="table table-sm">
+            <table class="table table-sm" aria-labelledby="objectsPerYear">


I'm still concerned about these objectsPerYear labbeledbys that have no id matching objectsPerYear. (and similar, on many tables).

kaladay · 2025-01-24T22:18:02Z

src/app/shared/dialog/dialog.component.html

@@ -1,12 +1,12 @@
-<form novalidate>
+<form novalidate aria-labelledby="modal-basic-title" role="dialog" aria-modal="true">


Another case where I have concerns of the id being duplicated due to the possibility of multiple dialogs on a single page.

src/app/shared/sidebar/sidebar.component.html

Co-authored-by: Kevin Day <[email protected]>

…AMULib/scholars-angular into sprint9-410-accessibility-improvements

kaladay

Githubs interface is confusing me.
I think you changed all of my requests.

Dbreck-TAMU and others added 2 commits January 21, 2025 13:30

Updated Accessibility

c4027ee

Shared components accessibility improvements

84c224f

Dbreck-TAMU self-assigned this Jan 22, 2025

Dbreck-TAMU linked an issue Jan 22, 2025 that may be closed by this pull request

Improve Accessibility Compliance on Scholars Application #410

Open

Dbreck-TAMU and others added 10 commits January 22, 2025 15:23

header accessibility improvements

06d69e6

footer accessibility improvements

589feba

visualization accessibility improvements

76316df

display accessibility improvements

35a5cd1

discovery accessibility improvements

cd3dd1b

directory accessibility improvements

7a1dffa

Accessibility fixes for Angular

9b00bfc

about component accessibility improvements

242f315

410: Manually override pagination colors to enforce color contrast ra…

59c5735

…tio. The https://webaim.org/resources/contrastchecker/ is used to determine valid color contrast ratios.

Merge pull request #423 from TAMULib/sprint9-410-accessibility-improv…

8f3c72b

…ements-color_contrast [Issue 410] Accessibility improvements color contrast.

Dbreck-TAMU requested a review from kaladay January 24, 2025 18:34

Dbreck-TAMU marked this pull request as ready for review January 24, 2025 18:34

Dbreck-TAMU and others added 13 commits January 24, 2025 12:38

Merge branch 'sprint-9-staging' into sprint9-410-accessibility-improv…

101e5b4

…ements

Reapplied merge issues

16d2437

updated aria-labels to be translatable

b659d97

Removed labels for explicity named <a> tags

682c5da

Removed aria labels for text based links

ebd6636

Merge branch 'sprint9-410-accessibility-improvements' of github.com:T…

9f3c86d

…AMULib/scholars-angular into sprint9-410-accessibility-improvements

Corrected footer expression

90ab69f

accessibility improvements

fb7c289

removed extraneous aria labels

65a0365

removed extraneous aria labels

9d649af

Removed extraneous aria labels

c2d6306

Merge pull request #424 from TAMULib/sprint9-410-accessibility-improv…

338afcd

…ements-color_contrast-2 410: Manually override pagination colors to enforce color contrast ratio.

kaladay requested changes Jan 24, 2025

View reviewed changes

Update src/app/+directory/directory.component.html

66cd7de

Co-authored-by: Kevin Day <[email protected]>

kaladay requested changes Jan 24, 2025

View reviewed changes

Dbreck-TAMU added 2 commits January 24, 2025 16:06

Adjusted accessibility items

bc7b232

Removed re-useable accessibility label

984a421

kaladay requested changes Jan 24, 2025

View reviewed changes

kaladay reviewed Jan 24, 2025

View reviewed changes

Correct bad merge for tab.component

ad4e969

kaladay reviewed Jan 24, 2025

View reviewed changes

removed potential label duplicates

70eea01

kaladay reviewed Jan 24, 2025

View reviewed changes

src/app/shared/sidebar/sidebar.component.html Outdated Show resolved Hide resolved

Dbreck-TAMU and others added 5 commits January 24, 2025 16:40

Update src/app/shared/sidebar/sidebar.component.html

7035cd9

Co-authored-by: Kevin Day <[email protected]>

removed labelledby property

a558c0b

Merge branch 'sprint9-410-accessibility-improvements' of github.com:T…

ccb13de

…AMULib/scholars-angular into sprint9-410-accessibility-improvements

Removed non-unique accessibility properties

1bdf5a2

Removed extraneous labels

3c4b15c

kaladay approved these changes Jan 24, 2025

View reviewed changes

Dbreck-TAMU merged commit 91766b7 into sprint-9-staging Jan 24, 2025

Dbreck-TAMU deleted the sprint9-410-accessibility-improvements branch January 31, 2025 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 410 accessibility improvements #416

Issue 410 accessibility improvements #416

Dbreck-TAMU commented Jan 22, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay left a comment

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025 •

edited

Loading

kaladay Jan 24, 2025 •

edited

Loading

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay Jan 24, 2025

kaladay left a comment

		@@ -1,9 +1,7 @@
		<div *ngIf="individual \| async; let individual" class="tab-content">
		<div *ngIf="tab \| async; let tab" class="tab-pane active">

		@@ -1,12 +1,12 @@
		<form novalidate>
		<form novalidate aria-labelledby="modal-basic-title" role="dialog" aria-modal="true">

Issue 410 accessibility improvements #416

Issue 410 accessibility improvements #416

Conversation

Dbreck-TAMU commented Jan 22, 2025

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaladay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaladay Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

kaladay Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaladay left a comment

Choose a reason for hiding this comment

kaladay Jan 24, 2025 •

edited

Loading

kaladay Jan 24, 2025 •

edited

Loading