117544: PR feedback

Jens Vannerum · Jens Vannerum · commit 9fd3c2d73480 · 2024-09-16T13:44:06.000+02:00
- added typedocs
- changed directive to only be present for buttons
- various other small fixes
diff --git a/src/app/admin/admin-reports/filtered-items/filtered-items.component.html b/src/app/admin/admin-reports/filtered-items/filtered-items.component.html
@@ -12,7 +12,7 @@ <h1 id="header" class="border-bottom pb-2">{{'admin.reports.items.head' | transl
                         </ng-template>
                         <ng-template ngbPanelContent>
                             <select id="collSel" name="collSel" class="form-control" multiple="multiple" size="10" formControlName="collections">
-                                <option *ngFor="let item of collections" [value]="item.id" [dsDisabled]="item.disabled">{{item.name$ | async}}</option>
+                                <option *ngFor="let item of collections" [value]="item.id" [disabled]="item.disabled">{{item.name$ | async}}</option>
                             </select>
                             <div class="row">
                                 <span class="col-3"></span>
diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.html b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.html
@@ -36,7 +36,7 @@
           [authorityValue]="mdValue.newValue.confidence"
           [iconMode]="true"
       ></i>
-        <input class="form-control form-outline" data-test="authority-input" [(ngModel)]="mdValue.newValue.authority" [dsDisabled]="!editingAuthority"
+        <input class="form-control form-outline" data-test="authority-input" [(ngModel)]="mdValue.newValue.authority" [disabled]="!editingAuthority"
               [attr.aria-label]="(dsoType + '.edit.metadata.edit.authority.key') | translate"
               (change)="onChangeAuthorityKey()" />
         <button class="btn btn-outline-secondary btn-sm ng-star-inserted" id="metadata-confirm-btn" *ngIf="!editingAuthority"
diff --git a/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.spec.ts b/src/app/dso-shared/dso-edit-metadata/dso-edit-metadata-value/dso-edit-metadata-value.component.spec.ts
@@ -462,8 +462,7 @@ describe('DsoEditMetadataValueComponent', () => {
         fixture.detectChanges();
         fixture.whenStable().then(() => {
           const inputElement = fixture.nativeElement.querySelector('input[data-test="authority-input"]');
-          expect(inputElement.getAttribute('aria-disabled')).toBe('true');
-          expect(inputElement.classList.contains('disabled')).toBeTrue();
+          expect(inputElement.disabled).toBeTruthy();
           done();
         });
       });
diff --git a/src/app/item-page/edit-item-page/item-delete/item-delete.component.html b/src/app/item-page/edit-item-page/item-delete/item-delete.component.html
@@ -18,7 +18,7 @@ <h1>{{headerMessage | translate: {id: item.handle} }}</h1>
 
                             <div class="m-2" (click)="setSelected(typeDto.relationshipType, !selected)">
                                 <label>
-                                    <input type="checkbox" [checked]="selected" [dsDisabled]="isDeleting$ | async">
+                                    <input type="checkbox" [checked]="selected" [disabled]="isDeleting$ | async">
                                 </label>
                             </div>
 
diff --git a/src/app/shared/access-control-form-container/access-control-array-form/access-control-array-form.component.html b/src/app/shared/access-control-form-container/access-control-array-form/access-control-array-form.component.html
@@ -13,7 +13,7 @@
           </label>
           <select
             id="accesscontroloption-{{control.id}}"
-            [dsDisabled]="ngForm.disabled"
+            [disabled]="ngForm.disabled"
             [(ngModel)]="control.itemName"
             (ngModelChange)="accessControlChanged(control, $event)"
             name="itemName-{{control.id}}"
diff --git a/src/app/shared/access-control-form-container/access-control-form-container.component.html b/src/app/shared/access-control-form-container/access-control-form-container.component.html
@@ -31,7 +31,7 @@ <h2 class="h3 mb-0 mr-4">
                 <div class="form-check">
                   <input class="form-check-input" type="radio"
                          name="itemMode" id="itemReplace" value="replace"
-                         [dsDisabled]="!state.item.toggleStatus"
+                         [disabled]="!state.item.toggleStatus"
                          [(ngModel)]="state.item.accessMode">
                   <label class="form-check-label" for="itemReplace">
                     {{'access-control-replace-all' | translate}}
@@ -40,7 +40,7 @@ <h2 class="h3 mb-0 mr-4">
                 <div class="form-check">
                   <input class="form-check-input" type="radio"
                          name="itemMode" id="itemAdd" value="add"
-                         [dsDisabled]="!state.item.toggleStatus"
+                         [disabled]="!state.item.toggleStatus"
                          [(ngModel)]="state.item.accessMode">
                   <label class="form-check-label" for="itemAdd">
                     {{'access-control-add-to-existing' | translate}}
@@ -85,7 +85,7 @@ <h2 class="h3 mb-0 mr-4">
               <div class="form-check">
                 <input class="form-check-input" type="radio"
                        name="changesLimit" id="processAll" value="all"
-                       [dsDisabled]="!state.bitstream.toggleStatus"
+                       [disabled]="!state.bitstream.toggleStatus"
                        [(ngModel)]="state.bitstream.changesLimit">
                 <label class="form-check-label" for="processAll">
                   {{'access-control-process-all-bitstreams' | translate}}
@@ -94,7 +94,7 @@ <h2 class="h3 mb-0 mr-4">
               <div class="form-check">
                 <input class="form-check-input mt-2" type="radio"
                        name="changesLimit" id="processSelected" value="selected"
-                       [dsDisabled]="!state.bitstream.toggleStatus"
+                       [disabled]="!state.bitstream.toggleStatus"
                        [(ngModel)]="state.bitstream.changesLimit">
                 <label class="form-check-label" for="processSelected">
                   {{ state.bitstream.selectedBitstreams.length }}
@@ -123,7 +123,7 @@ <h2 class="h3 mb-0 mr-4">
               <div class="form-check">
                 <input class="form-check-input" type="radio"
                        name="bitstreamMode" id="bitstreamReplace" value="replace"
-                       [dsDisabled]="!state.bitstream.toggleStatus"
+                       [disabled]="!state.bitstream.toggleStatus"
                        [(ngModel)]="state.bitstream.accessMode">
                 <label class="form-check-label" for="bitstreamReplace">
                   {{'access-control-replace-all' | translate}}
@@ -132,7 +132,7 @@ <h2 class="h3 mb-0 mr-4">
               <div class="form-check">
                 <input class="form-check-input" type="radio"
                        name="bitstreamMode" id="bitstreamAdd" value="add"
-                       [dsDisabled]="!state.bitstream.toggleStatus"
+                       [disabled]="!state.bitstream.toggleStatus"
                        [(ngModel)]="state.bitstream.accessMode">
                 <label class="form-check-label" for="bitstreamAdd">
                   {{'access-control-add-to-existing' | translate}}
diff --git a/src/app/shared/disabled-directive.spec.ts b/src/app/shared/disabled-directive.spec.ts
@@ -48,6 +48,14 @@ describe('DisabledDirective', () => {
     expect(button.nativeElement.classList.contains('disabled')).toBeTrue();
   });
 
+  it('should bind aria-disabled to false and not have disabled class when isDisabled is false', () => {
+    component.isDisabled = false;
+    fixture.detectChanges();
+
+    expect(button.nativeElement.getAttribute('aria-disabled')).toBe('false');
+    expect(button.nativeElement.classList.contains('disabled')).toBeFalse();
+  });
+
   it('should prevent click events when disabled', () => {
     component.isDisabled = true;
     fixture.detectChanges();
diff --git a/src/app/shared/disabled-directive.ts b/src/app/shared/disabled-directive.ts
@@ -9,15 +9,36 @@ import {
   selector: '[dsDisabled]',
   standalone: true,
 })
+
+/**
+ * This directive can be added to a html element to disable it (make it non-interactive).
+ * It acts as a replacement for HTML's disabled attribute.
+ *
+ * This directive should always be used instead of the HTML disabled attribute as it is more accessible.
+ */
 export class DisabledDirective {
 
     @Input() set dsDisabled(value: boolean) {
     this.isDisabled = !!value;
   }
 
+    /**
+     * Binds the aria-disabled attribute to the directive's isDisabled property.
+     * This is used to make the element accessible to screen readers. If the element is disabled, the screen reader will announce it as such.
+     */
     @HostBinding('attr.aria-disabled') isDisabled = false;
+
+    /**
+     * Binds the class attribute to the directive's isDisabled property.
+     * This is used to style the element when it is disabled (make it look disabled).
+     */
     @HostBinding('class.disabled') get disabledClass() { return this.isDisabled; }
 
+    /**
+     * Prevents the default action and stops the event from propagating when the element is disabled.
+     * This is used to prevent the element from being interacted with when it is disabled (via mouse click).
+     * @param event The click event.
+     */
     @HostListener('click', ['$event'])
     handleClick(event: Event) {
       if (this.isDisabled) {
@@ -26,7 +47,12 @@ export class DisabledDirective {
       }
     }
 
-    @HostListener('keydown', ['$event'])
+    /**
+     * Prevents the default action and stops the event from propagating when the element is disabled.
+     * This is used to prevent the element from being interacted with when it is disabled (via keystrokes).
+     * @param event The keydown event.
+     */
+  @HostListener('keydown', ['$event'])
     handleKeydown(event: KeyboardEvent) {
       if (this.isDisabled && (event.key === 'Enter' || event.key === 'Space')) {
         event.preventDefault();
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html
@@ -31,7 +31,7 @@
     <div *ngIf="model.languageCodes && model.languageCodes.length > 0" class="col-xs-2" >
       <select
         #language="ngModel"
-        [dsDisabled]="model.readOnly"
+        [disabled]="model.readOnly"
         [(ngModel)]="model.language"
         class="form-control"
         (blur)="onBlur($event)"
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html
@@ -6,7 +6,7 @@
     <ds-number-picker
       tabindex="0"
       [id]="model.id + '_year'"
-      [dsDisabled]="model.disabled"
+      [disabled]="model.disabled"
       [min]="minYear"
       [max]="maxYear"
       [name]="'year'"
@@ -30,7 +30,7 @@
       [(ngModel)]="initialMonth"
       [value]="month"
       [placeholder]="monthPlaceholder"
-      [dsDisabled]="!year || model.disabled"
+      [disabled]="!year || model.disabled"
       (blur)="onBlur($event)"
       (change)="onChange($event)"
       (focus)="onFocus($event)"
@@ -46,7 +46,7 @@
       [(ngModel)]="initialDay"
       [value]="day"
       [placeholder]="dayPlaceholder"
-      [dsDisabled]="!month || model.disabled"
+      [disabled]="!month || model.disabled"
       (blur)="onBlur($event)"
       (change)="onChange($event)"
       (focus)="onFocus($event)"
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/disabled/dynamic-disabled.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/disabled/dynamic-disabled.component.html
@@ -6,7 +6,7 @@
                  [id]="id"
                  [name]="model.name"
                  [value]="modelValuesString"
-                 [dsDisabled]="model.disabled"
+                 [disabled]="model.disabled"
                  [type]="model.inputType"
                  [placeholder]="model.placeholder | translate"
                  [readonly]="model.readOnly">
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/disabled/dynamic-disabled.component.spec.ts b/src/app/shared/form/builder/ds-dynamic-form-ui/models/disabled/dynamic-disabled.component.spec.ts
@@ -75,7 +75,7 @@ describe('DsDynamicDisabledComponent', () => {
     expect(comp).toBeTruthy();
   });
 
-  xit('should have a disabled input', () => {
+  it('should have a disabled input', () => {
     const input = de.query(By.css('input'));
     expect(input.nativeElement.getAttribute('disabled')).toEqual('');
   });
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html
@@ -18,7 +18,7 @@
              [name]="model.name"
              [type]="model.inputType"
              [(ngModel)]="firstInputValue"
-             [dsDisabled]="isInputDisabled()"
+             [disabled]="isInputDisabled()"
              [placeholder]="model.placeholder | translate"
              [readonly]="model.readOnly"
              (change)="onChange($event)"
@@ -38,7 +38,7 @@
               [name]="model.name + '_2'"
               [type]="model.inputType"
               [(ngModel)]="secondInputValue"
-              [dsDisabled]="firstInputValue.length === 0 || isInputDisabled()"
+              [disabled]="firstInputValue.length === 0 || isInputDisabled()"
               [placeholder]="model.secondPlaceholder | translate"
               [readonly]="model.readOnly"
               (change)="onChange($event)"
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/onebox/dynamic-onebox.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/onebox/dynamic-onebox.component.html
@@ -40,7 +40,7 @@
          [ngbTypeahead]="search"
          [placeholder]="model.placeholder"
          [readonly]="model.readOnly"
-         [dsDisabled]="model.readOnly"
+         [disabled]="model.readOnly"
          [resultTemplate]="rt"
          [type]="model.inputType"
          [(ngModel)]="currentValue"
@@ -66,7 +66,7 @@
          [name]="model.name"
          [placeholder]="model.placeholder"
          [readonly]="true"
-         [dsDisabled]="model.readOnly"
+         [disabled]="model.readOnly"
          [type]="model.inputType"
          [value]="currentValue?.display"
          (focus)="onFocus($event)"
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/scrollable-dropdown/dynamic-scrollable-dropdown.component.html
@@ -18,7 +18,7 @@
            [id]="id"
            [name]="model.name"
            [readonly]="true"
-           [dsDisabled]="model.readOnly"
+           [disabled]="model.readOnly"
            [type]="model.inputType"
            [value]="(currentValue | async)"
            (blur)="onBlur($event)"
diff --git a/src/app/shared/form/number-picker/number-picker.component.html b/src/app/shared/form/number-picker/number-picker.component.html
@@ -21,7 +21,7 @@
       (change)="update($event); $event.stopPropagation();"
       (focus)="onFocus($event); $event.stopPropagation();"
       [readonly]="disabled"
-      [dsDisabled]="disabled"
+      [disabled]="disabled"
       [ngClass]="{'is-invalid': invalid}"
       title="{{placeholder}}"
       [attr.aria-label]="placeholder"
diff --git a/src/app/shared/form/vocabulary-treeview/vocabulary-treeview.component.html b/src/app/shared/form/vocabulary-treeview/vocabulary-treeview.component.html
@@ -40,7 +40,7 @@ <h2 *ngIf="(loading | async) !== true && dataSource.data.length === 0" class="h4
                  container="body"
           >
             <input class="mr-2" type="checkbox"
-                   [dsDisabled]="!node.item?.selectable"
+                   [disabled]="!node.item?.selectable"
                    [(ngModel)]="node.isSelected"
                    [checked]="node.isSelected"
                    (change)="onSelect(node.item)"
@@ -72,7 +72,7 @@ <h2 *ngIf="(loading | async) !== true && dataSource.data.length === 0" class="h4
                 [openDelay]="500"
                 container="body">
             <input class="mr-2" type="checkbox"
-                   [dsDisabled]="!node.item?.selectable"
+                   [disabled]="!node.item?.selectable"
                    [(ngModel)]="node.isSelected"
                    [checked]="node.isSelected"
                    (change)="onSelect(node.item)"
diff --git a/src/app/shared/object-select/collection-select/collection-select.component.html b/src/app/shared/object-select/collection-select/collection-select.component.html
@@ -16,7 +16,7 @@
         </thead>
         <tbody>
           <tr *ngFor="let selectCollection of selectCollections$ | async">
-            <td><input #selectCollectionBtn [attr.aria-label]="(selectCollectionBtn.checked ? 'collection.select.table.deselect' : 'collection.select.table.select') | translate" [dsDisabled]="(selectCollection.canSelect$ | async) === false" class="collection-checkbox" [ngModel]="selectCollection.selected$ | async" (change)="switch(selectCollection.dso.id)" type="checkbox" name="{{selectCollection.dso.id}}"></td>
+            <td><input #selectCollectionBtn [attr.aria-label]="(selectCollectionBtn.checked ? 'collection.select.table.deselect' : 'collection.select.table.select') | translate" [disabled]="(selectCollection.canSelect$ | async) === false" class="collection-checkbox" [ngModel]="selectCollection.selected$ | async" (change)="switch(selectCollection.dso.id)" type="checkbox" name="{{selectCollection.dso.id}}"></td>
             <td><a [routerLink]="selectCollection.route">{{ dsoNameService.getName(selectCollection.dso) }}</a></td>
           </tr>
         </tbody>
diff --git a/src/app/shared/object-select/item-select/item-select.component.html b/src/app/shared/object-select/item-select/item-select.component.html
@@ -18,7 +18,7 @@
         </thead>
         <tbody>
         <tr *ngFor="let selectItem of selectItems$ | async">
-          <td><input #selectItemBtn [attr.aria-label]="(selectItemBtn.checked ? 'item.select.table.deselect' : 'item.select.table.select') | translate" [dsDisabled]="(selectItem.canSelect$ | async) === false" class="item-checkbox" [ngModel]="selectItem.selected$ | async" (change)="switch(selectItem.dso.id)" type="checkbox" name="{{selectItem.dso.id}}"></td>
+          <td><input #selectItemBtn [attr.aria-label]="(selectItemBtn.checked ? 'item.select.table.deselect' : 'item.select.table.select') | translate" [disabled]="(selectItem.canSelect$ | async) === false" class="item-checkbox" [ngModel]="selectItem.selected$ | async" (change)="switch(selectItem.dso.id)" type="checkbox" name="{{selectItem.dso.id}}"></td>
           <td *ngIf="!hideCollection">
             <span *ngVar="(selectItem.dso.owningCollection | async)?.payload as collection">
               <a *ngIf="collection" [routerLink]="['/collections', collection?.id]">
diff --git a/src/app/shared/object-select/item-select/item-select.component.spec.ts b/src/app/shared/object-select/item-select/item-select.component.spec.ts
@@ -197,8 +197,7 @@ describe('ItemSelectComponent', () => {
 
       const checkbox = fixture.debugElement.query(By.css('input.item-checkbox')).nativeElement;
       expect(authorizationDataService.isAuthorized).toHaveBeenCalled();
-      expect(checkbox.getAttribute('aria-disabled')).toBe('true');
-      expect(checkbox.classList.contains('disabled')).toBe(true);
+      expect(checkbox.disabled).toBeTrue();
     }));
   });
 });
diff --git a/src/app/shared/pagination/pagination.component.html b/src/app/shared/pagination/pagination.component.html
@@ -41,7 +41,7 @@
     <div *ngIf="showPaginator" class="pagination justify-content-center clearfix bottom">
       <ngb-pagination [boundaryLinks]="paginationOptions.boundaryLinks"
                       [collectionSize]="collectionSize"
-                      [dsDisabled]="paginationOptions.disabled"
+                      [disabled]="paginationOptions.disabled"
                       [ellipses]="paginationOptions.ellipses"
                       [maxSize]="(isXs)?5:paginationOptions.maxSize"
                       [page]="(currentPage$|async)"
diff --git a/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.html b/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.html
@@ -1,6 +1,6 @@
 <div class="mb-4 ccLicense-select">
   <ds-select
-    [dsDisabled]="!submissionCcLicenses">
+    [disabled]="!submissionCcLicenses">
 
     <ng-container class="selection">
       <span *ngIf="!submissionCcLicenses">
@@ -81,7 +81,7 @@ <h5>
 
     </ng-template>
 
-    <ds-select *ngIf="field.enums?.length > 5" [dsDisabled]="field.id === 'jurisdiction' && defaultJurisdiction !== undefined && defaultJurisdiction !== 'none'">
+    <ds-select *ngIf="field.enums?.length > 5" [disabled]="field.id === 'jurisdiction' && defaultJurisdiction !== undefined && defaultJurisdiction !== 'none'">
       <ng-container class="selection" *ngVar="getSelectedOption(getSelectedCcLicense(), field) as option">
         <span *ngIf="option">
           {{ option.label }}
diff --git a/src/app/submission/sections/upload/file/section-upload-file.component.html b/src/app/submission/sections/upload/file/section-upload-file.component.html
@@ -7,7 +7,7 @@
         type="checkbox"
         class="custom-control-input"
         id="primaryBitstream{{fileIndex}}"
-        [dsDisabled]="processingSaveStatus$ | async"
+        [disabled]="processingSaveStatus$ | async"
         [checked]="isPrimary"
         (change)="togglePrimaryBitstream($event)">
         <label class="custom-control-label" for="primaryBitstream{{fileIndex}}">
