Skip to content

Commit

Permalink
Merge pull request DSpace#3268 from atmire/w2p-117544_support-for-dis…
Browse files Browse the repository at this point in the history
…abled-elements-for-screen-readers-9.0

support for disabled buttons for screen readers
  • Loading branch information
tdonohue authored Jan 28, 2025
2 parents df37780 + 920edef commit d70fa8f
Show file tree
Hide file tree
Showing 188 changed files with 866 additions and 236 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@
],
"rules": {
// Custom DSpace Angular rules
"dspace-angular-html/themed-component-usages": "error"
"dspace-angular-html/themed-component-usages": "error",
"dspace-angular-html/no-disabled-attribute-on-button": "error"
}
},
{
Expand Down
1 change: 1 addition & 0 deletions docs/lint/html/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
_______

- [`dspace-angular-html/themed-component-usages`](./rules/themed-component-usages.md): Themeable components should be used via the selector of their `ThemedComponent` wrapper class
- [`dspace-angular-html/no-disabled-attribute-on-button`](./rules/no-disabled-attribute-on-button.md): Buttons should use the `dsBtnDisabled` directive instead of the HTML `disabled` attribute.
78 changes: 78 additions & 0 deletions docs/lint/html/rules/no-disabled-attribute-on-button.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
[DSpace ESLint plugins](../../../../lint/README.md) > [HTML rules](../index.md) > `dspace-angular-html/no-disabled-attribute-on-button`
_______

Buttons should use the `dsBtnDisabled` directive instead of the HTML `disabled` attribute.
This should be done to ensure that users with a screen reader are able to understand that the a button button is present, and that it is disabled.
The native html disabled attribute does not allow users to navigate to the button by keyboard, and thus they have no way of knowing that the button is present.

_______

[Source code](../../../../lint/src/rules/html/no-disabled-attribute-on-button.ts)

### Examples


#### Valid code

##### should use [dsBtnDisabled] in HTML templates
```html
<button [dsBtnDisabled]="true">Submit</button>
```

##### disabled attribute is still valid on non-button elements
```html
<input disabled>
```

##### [disabled] attribute is still valid on non-button elements
```html
<input [disabled]="true">
```

##### angular dynamic attributes that use disabled are still valid
```html
<button [class.disabled]="isDisabled">Submit</button>
```




#### Invalid code &amp; automatic fixes

##### should not use disabled attribute in HTML templates
```html
<button disabled>Submit</button>
```
Will produce the following error(s):
```
Buttons should use the `dsBtnDisabled` directive instead of the `disabled` attribute.
```
Result of `yarn lint --fix`:
```html
<button [dsBtnDisabled]="true">Submit</button>
```

##### should not use [disabled] attribute in HTML templates
```html
<button [disabled]="true">Submit</button>
```
Will produce the following error(s):
```
Buttons should use the `dsBtnDisabled` directive instead of the `disabled` attribute.
```
Result of `yarn lint --fix`:
```html
<button [dsBtnDisabled]="true">Submit</button>
```


3 changes: 3 additions & 0 deletions lint/src/rules/html/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import {
bundle,
RuleExports,
} from '../../util/structure';
import * as noDisabledAttributeOnButton from './no-disabled-attribute-on-button';
import * as themedComponentUsages from './themed-component-usages';

const index = [
themedComponentUsages,
noDisabledAttributeOnButton,

] as unknown as RuleExports[];

export = {
Expand Down
147 changes: 147 additions & 0 deletions lint/src/rules/html/no-disabled-attribute-on-button.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import {
TmplAstBoundAttribute,
TmplAstTextAttribute,
} from '@angular-eslint/bundled-angular-compiler';
import { TemplateParserServices } from '@angular-eslint/utils';
import {
ESLintUtils,
TSESLint,
} from '@typescript-eslint/utils';

import {
DSpaceESLintRuleInfo,
NamedTests,
} from '../../util/structure';
import { getSourceCode } from '../../util/typescript';

export enum Message {
USE_DSBTN_DISABLED = 'mustUseDsBtnDisabled',
}

export const info = {
name: 'no-disabled-attribute-on-button',
meta: {
docs: {
description: `Buttons should use the \`dsBtnDisabled\` directive instead of the HTML \`disabled\` attribute.
This should be done to ensure that users with a screen reader are able to understand that the a button button is present, and that it is disabled.
The native html disabled attribute does not allow users to navigate to the button by keyboard, and thus they have no way of knowing that the button is present.`,
},
type: 'problem',
fixable: 'code',
schema: [],
messages: {
[Message.USE_DSBTN_DISABLED]: 'Buttons should use the `dsBtnDisabled` directive instead of the `disabled` attribute.',
},
},
defaultOptions: [],
} as DSpaceESLintRuleInfo;

export const rule = ESLintUtils.RuleCreator.withoutDocs({
...info,
create(context: TSESLint.RuleContext<Message, unknown[]>) {
const parserServices = getSourceCode(context).parserServices as TemplateParserServices;

/**
* Some dynamic angular inputs will have disabled as name because of how Angular handles this internally (e.g [class.disabled]="isDisabled")
* But these aren't actually the disabled attribute we're looking for, we can determine this by checking the details of the keySpan
*/
function isOtherAttributeDisabled(node: TmplAstBoundAttribute | TmplAstTextAttribute): boolean {
// if the details are not null, and the details are not 'disabled', then it's not the disabled attribute we're looking for
return node.keySpan?.details !== null && node.keySpan?.details !== 'disabled';
}

/**
* Replace the disabled text with [dsBtnDisabled] in the template
*/
function replaceDisabledText(text: string ): string {
const hasBrackets = text.includes('[') && text.includes(']');
const newDisabledText = hasBrackets ? 'dsBtnDisabled' : '[dsBtnDisabled]="true"';
return text.replace('disabled', newDisabledText);
}

function inputIsChildOfButton(node: any): boolean {
return (node.parent?.tagName === 'button' || node.parent?.name === 'button');
}

function reportAndFix(node: TmplAstBoundAttribute | TmplAstTextAttribute) {
if (!inputIsChildOfButton(node) || isOtherAttributeDisabled(node)) {
return;
}

const sourceSpan = node.sourceSpan;
context.report({
messageId: Message.USE_DSBTN_DISABLED,
loc: parserServices.convertNodeSourceSpanToLoc(sourceSpan),
fix(fixer) {
const templateText = sourceSpan.start.file.content;
const disabledText = templateText.slice(sourceSpan.start.offset, sourceSpan.end.offset);
const newText = replaceDisabledText(disabledText);
return fixer.replaceTextRange([sourceSpan.start.offset, sourceSpan.end.offset], newText);
},
});
}

return {
'BoundAttribute[name="disabled"]'(node: TmplAstBoundAttribute) {
reportAndFix(node);
},
'TextAttribute[name="disabled"]'(node: TmplAstTextAttribute) {
reportAndFix(node);
},
};
},
});

export const tests = {
plugin: info.name,
valid: [
{
name: 'should use [dsBtnDisabled] in HTML templates',
code: `
<button [dsBtnDisabled]="true">Submit</button>
`,
},
{
name: 'disabled attribute is still valid on non-button elements',
code: `
<input disabled>
`,
},
{
name: '[disabled] attribute is still valid on non-button elements',
code: `
<input [disabled]="true">
`,
},
{
name: 'angular dynamic attributes that use disabled are still valid',
code: `
<button [class.disabled]="isDisabled">Submit</button>
`,
},
],
invalid: [
{
name: 'should not use disabled attribute in HTML templates',
code: `
<button disabled>Submit</button>
`,
errors: [{ messageId: Message.USE_DSBTN_DISABLED }],
output: `
<button [dsBtnDisabled]="true">Submit</button>
`,
},
{
name: 'should not use [disabled] attribute in HTML templates',
code: `
<button [disabled]="true">Submit</button>
`,
errors: [{ messageId: Message.USE_DSBTN_DISABLED }],
output: `
<button [dsBtnDisabled]="true">Submit</button>
`,
},
],
} as NamedTests;

export default rule;
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ <h1>{{ 'admin.access-control.bulk-access.title' | translate }}</h1>
<button class="btn btn-outline-primary mr-3" (click)="reset()">
{{ 'access-control-cancel' | translate }}
</button>
<button class="btn btn-primary" [disabled]="!canExport()" (click)="submit()">
<button class="btn btn-primary" [dsBtnDisabled]="!canExport()" (click)="submit()">
{{ 'access-control-execute' | translate }}
</button>
</div>
Expand Down
2 changes: 2 additions & 0 deletions src/app/access-control/bulk-access/bulk-access.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from 'rxjs/operators';

import { BulkAccessControlService } from '../../shared/access-control-form-container/bulk-access-control.service';
import { BtnDisabledDirective } from '../../shared/btn-disabled.directive';
import { SelectableListState } from '../../shared/object-list/selectable-list/selectable-list.reducer';
import { SelectableListService } from '../../shared/object-list/selectable-list/selectable-list.service';
import { BulkAccessBrowseComponent } from './browse/bulk-access-browse.component';
Expand All @@ -27,6 +28,7 @@ import { BulkAccessSettingsComponent } from './settings/bulk-access-settings.com
TranslateModule,
BulkAccessSettingsComponent,
BulkAccessBrowseComponent,
BtnDisabledDirective,
],
standalone: true,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { EPersonDataService } from '../../core/eperson/eperson-data.service';
import { EPerson } from '../../core/eperson/models/eperson.model';
import { PaginationService } from '../../core/pagination/pagination.service';
import { PageInfo } from '../../core/shared/page-info.model';
import { BtnDisabledDirective } from '../../shared/btn-disabled.directive';
import { FormBuilderService } from '../../shared/form/builder/form-builder.service';
import { ThemedLoadingComponent } from '../../shared/loading/themed-loading.component';
import { getMockFormBuilderService } from '../../shared/mocks/form-builder-service.mock';
Expand Down Expand Up @@ -151,7 +152,7 @@ describe('EPeopleRegistryComponent', () => {
paginationService = new PaginationServiceStub();
TestBed.configureTestingModule({
imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule, RouterTestingModule.withRoutes([]),
TranslateModule.forRoot(), EPeopleRegistryComponent],
TranslateModule.forRoot(), EPeopleRegistryComponent, BtnDisabledDirective],
providers: [
{ provide: EPersonDataService, useValue: ePersonDataServiceStub },
{ provide: NotificationsService, useValue: new NotificationsServiceStub() },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ <h1 class="border-bottom pb-2">{{messagePrefix + '.edit' | translate}}</h1>
</button>
</div>
<div *ngIf="displayResetPassword" between class="btn-group">
<button class="btn btn-primary" [disabled]="(canReset$ | async) !== true" type="button" (click)="resetPassword()">
<button class="btn btn-primary" [dsBtnDisabled]="(canReset$ | async) !== true" type="button" (click)="resetPassword()">
<i class="fa fa-key"></i> {{'admin.access-control.epeople.actions.reset' | translate}}
</button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { GroupDataService } from '../../../core/eperson/group-data.service';
import { EPerson } from '../../../core/eperson/models/eperson.model';
import { PaginationService } from '../../../core/pagination/pagination.service';
import { PageInfo } from '../../../core/shared/page-info.model';
import { BtnDisabledDirective } from '../../../shared/btn-disabled.directive';
import { FormBuilderService } from '../../../shared/form/builder/form-builder.service';
import { FormComponent } from '../../../shared/form/form.component';
import { ThemedLoadingComponent } from '../../../shared/loading/themed-loading.component';
Expand Down Expand Up @@ -221,7 +222,7 @@ describe('EPersonFormComponent', () => {
route = new ActivatedRouteStub();
router = new RouterStub();
TestBed.configureTestingModule({
imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BtnDisabledDirective, BrowserModule,
RouterModule.forRoot([]),
TranslateModule.forRoot(),
EPersonFormComponent,
Expand Down Expand Up @@ -516,7 +517,8 @@ describe('EPersonFormComponent', () => {
// ePersonDataServiceStub.activeEPerson = eperson;
spyOn(component.epersonService, 'deleteEPerson').and.returnValue(createSuccessfulRemoteDataObject$('No Content', 204));
const deleteButton = fixture.debugElement.query(By.css('.delete-button'));
expect(deleteButton.nativeElement.disabled).toBe(false);
expect(deleteButton.nativeElement.getAttribute('aria-disabled')).toBeNull();
expect(deleteButton.nativeElement.classList.contains('disabled')).toBeFalse();
deleteButton.triggerEventHandler('click', null);
fixture.detectChanges();
expect(component.epersonService.deleteEPerson).toHaveBeenCalledWith(eperson);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
import { PageInfo } from '../../../core/shared/page-info.model';
import { Registration } from '../../../core/shared/registration.model';
import { TYPE_REQUEST_FORGOT } from '../../../register-email-form/register-email-form.component';
import { BtnDisabledDirective } from '../../../shared/btn-disabled.directive';
import { ConfirmationModalComponent } from '../../../shared/confirmation-modal/confirmation-modal.component';
import { hasValue } from '../../../shared/empty.util';
import { FormBuilderService } from '../../../shared/form/builder/form-builder.service';
Expand Down Expand Up @@ -92,6 +93,7 @@ import { ValidateEmailNotTaken } from './validators/email-taken.validator';
PaginationComponent,
RouterLink,
HasNoValuePipe,
BtnDisabledDirective,
],
standalone: true,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ <h3>{{messagePrefix + '.headMembers' | translate}}</h3>
<div class="btn-group edit-field">
<button (click)="deleteMemberFromGroup(epersonDTO.eperson)"
*ngIf="epersonDTO.ableToDelete"
[disabled]="actionConfig.remove.disabled"
[dsBtnDisabled]="actionConfig.remove.disabled"
[ngClass]="['btn btn-sm', actionConfig.remove.css]"
title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(epersonDTO.eperson) } }}">
<i [ngClass]="actionConfig.remove.icon"></i>
</button>
<button *ngIf="!epersonDTO.ableToDelete"
(click)="addMemberToGroup(epersonDTO.eperson)"
[disabled]="actionConfig.add.disabled"
[dsBtnDisabled]="actionConfig.add.disabled"
[ngClass]="['btn btn-sm', actionConfig.add.css]"
title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(epersonDTO.eperson) } }}">
<i [ngClass]="actionConfig.add.icon"></i>
Expand Down Expand Up @@ -122,7 +122,7 @@ <h3 id="search" class="border-bottom pb-2">
<td class="align-middle">
<div class="btn-group edit-field">
<button (click)="addMemberToGroup(eperson)"
[disabled]="actionConfig.add.disabled"
[dsBtnDisabled]="actionConfig.add.disabled"
[ngClass]="['btn btn-sm', actionConfig.add.css]"
title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(eperson) } }}">
<i [ngClass]="actionConfig.add.icon"></i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
getFirstCompletedRemoteData,
getRemoteDataPayload,
} from '../../../../core/shared/operators';
import { BtnDisabledDirective } from '../../../../shared/btn-disabled.directive';
import { ContextHelpDirective } from '../../../../shared/context-help.directive';
import { NotificationsService } from '../../../../shared/notifications/notifications.service';
import { PaginationComponent } from '../../../../shared/pagination/pagination.component';
Expand Down Expand Up @@ -113,6 +114,7 @@ export interface EPersonListActionConfig {
RouterLink,
NgClass,
NgForOf,
BtnDisabledDirective,
],
standalone: true,
})
Expand Down
Loading

0 comments on commit d70fa8f

Please sign in to comment.