CLARIN-DSpace v9/Restore per-instance DOM ids for ds-select and pool-task actions (X1 sweep, X-02a part A)

src/app/shared/ds-select/ds-select.component.html

@@ -3,13 +3,13 @@
   <div ngbDropdown class="btn-group" (openChange)="toggled.emit($event)">
 
     @if (label) {
-      <span  id="dsSelectMenuLabel" class="input-group-text">
+      <span  id="dsSelectMenuLabel-{{ uniqueId }}" class="input-group-text">
         {{ label | translate }}
       </span>
     }
 
-    <button aria-describedby="dsSelectMenuLabel"
-      id="dsSelectMenuButton"
+    <button [attr.aria-describedby]="label ? 'dsSelectMenuLabel-' + uniqueId : null"
+      id="dsSelectMenuButton-{{ uniqueId }}"
       class="btn btn-outline-primary selection"
       (blur)="close.emit($event)"
       (click)="close.emit($event)"
@@ -20,8 +20,8 @@
 
     <div ngbDropdownMenu
       class="dropdown-menu"
-      id="dsSelectDropdownMenu"
-      aria-labelledby="dsSelectMenuButton">
+      id="dsSelectDropdownMenu-{{ uniqueId }}"
+      [attr.aria-labelledby]="'dsSelectMenuButton-' + uniqueId">
       <div>
         <ng-content select=".menu"></ng-content>
       </div>

src/app/shared/ds-select/ds-select.component.spec.ts

@@ -1,3 +1,4 @@
+import { Component } from '@angular/core';
 import {
   ComponentFixture,
   TestBed,
@@ -7,6 +8,23 @@ import { TranslateModule } from '@ngx-translate/core';
 
 import { DsSelectComponent } from './ds-select.component';
 
+/**
+ * Two ds-select instances on one page - the situation that produced duplicate DOM ids
+ * (a browse toolbar renders one per sort option, MyDSpace one per pool task).
+ */
+@Component({
+  selector: 'ds-test-host',
+  template: `
+    <ds-select label="first.label"><span class="selection">A</span></ds-select>
+    <ds-select label="second.label"><span class="selection">B</span></ds-select>
+  `,
+  imports: [
+    DsSelectComponent,
+  ],
+})
+class TestHostComponent {
+}
+
 describe('DsSelectComponent', () => {
   let component: DsSelectComponent;
   let fixture: ComponentFixture<DsSelectComponent>;
@@ -30,4 +48,55 @@ describe('DsSelectComponent', () => {
   it('should create', () => {
     expect(component).toBeTruthy();
   });
+
+  it('should not reference a label element when no label is set', () => {
+    const button: HTMLElement = fixture.nativeElement.querySelector('button.selection');
+
+    expect(button).toBeTruthy();
+    expect(button.getAttribute('aria-describedby')).toBeNull();
+    // the button still names its own menu
+    const menu: HTMLElement = fixture.nativeElement.querySelector('[ngbDropdownMenu]');
+    expect(menu.getAttribute('aria-labelledby')).toEqual(button.id);
+  });
+
+  describe('with two instances on the same page', () => {
+    let hostFixture: ComponentFixture<TestHostComponent>;
+    let hostElement: HTMLElement;
+
+    beforeEach(() => {
+      hostFixture = TestBed.createComponent(TestHostComponent);
+      hostFixture.detectChanges();
+      hostElement = hostFixture.nativeElement;
+    });
+
+    it('should not emit duplicate DOM ids', () => {
+      const ids: string[] = Array.from(hostElement.querySelectorAll('[id]')).map((element: Element) => element.id);
+
+      expect(ids.length).toBeGreaterThan(0);
+      expect(ids.length).toEqual(new Set(ids).size);
+    });
+
+    it('should resolve every aria reference inside its own instance', () => {
+      const selects: HTMLElement[] = Array.from(hostElement.querySelectorAll('ds-select'));
+      expect(selects.length).toEqual(2);
+
+      const buttonIds: string[] = [];
+      selects.forEach((select: HTMLElement) => {
+        const button: HTMLElement = select.querySelector('button.selection');
+        const menu: HTMLElement = select.querySelector('[ngbDropdownMenu]');
+        const describedBy: string = button.getAttribute('aria-describedby');
+
+        expect(describedBy).toBeTruthy();
+        // the label the button points at is this instance's own label ...
+        expect(select.querySelector('[id="' + describedBy + '"]')).toBeTruthy();
+        // ... and no other element in the document answers to that id
+        expect(hostElement.querySelectorAll('[id="' + describedBy + '"]').length).toEqual(1);
+        expect(menu.getAttribute('aria-labelledby')).toEqual(button.id);
+
+        buttonIds.push(button.id);
+      });
+
+      expect(buttonIds[0]).not.toEqual(buttonIds[1]);
+    });
+  });
 });

src/app/shared/ds-select/ds-select.component.ts

@@ -10,6 +10,8 @@ import { TranslateModule } from '@ngx-translate/core';
 
 import { BtnDisabledDirective } from '../btn-disabled.directive';
 
+let nextDsSelectId = 0;
+
 /**
  * Component which represent a DSpace dropdown selector.
  */
@@ -25,6 +27,13 @@ import { BtnDisabledDirective } from '../btn-disabled.directive';
 })
 export class DsSelectComponent {
 
+  /**
+   * Unique identifier for the component instance. Several ds-select instances are rendered on the
+   * same page (browse toolbars, MyDSpace), so the dropdown's DOM ids have to be per-instance or the
+   * document carries duplicate ids and every aria reference resolves to the first instance.
+   */
+  uniqueId = `ds-select-${nextDsSelectId++}`;
+
   /**
    * An optional label for the dropdown selector.
    */

src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.html

@@ -19,7 +19,7 @@
   </ng-container>
   <div class="row">
     <div [ngClass]="showThumbnails ? 'offset-3 offset-md-2 col-9 col-md-10 ps-3' : ''">
-      <ds-pool-task-actions id="actions"
+      <ds-pool-task-actions id="actions-{{ dso?.id }}"
         [item]="item$.value"
         [object]="dso"
         [workflowitem]="workflowitem$.value"

src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.spec.ts

@@ -102,7 +102,7 @@ const environmentUseThumbs = {
 const rdItem = createSuccessfulRemoteDataObject(item);
 const workflowitem = Object.assign(new WorkflowItem(), { item: of(rdItem) });
 const rdWorkflowitem = createSuccessfulRemoteDataObject(workflowitem);
-mockResultObject.indexableObject = Object.assign(new PoolTask(), { workflowitem: of(rdWorkflowitem) });
+mockResultObject.indexableObject = Object.assign(new PoolTask(), { id: 'pool-task-1', workflowitem: of(rdWorkflowitem) });
 const linkService = getMockLinkService();
 const objectCacheServiceMock = jasmine.createSpyObj('ObjectCacheService', {
   remove: jasmine.createSpy('remove'),
@@ -171,4 +171,13 @@ describe('PoolSearchResultListElementComponent', () => {
     const thumbnail = fixture.debugElement.query(By.css('.offset-3'));
     expect(thumbnail).toBeTruthy();
   });
+
+  it('should give the pool task actions a row-specific id', () => {
+    const actions = fixture.debugElement.query(By.css('ds-pool-task-actions'));
+
+    expect(actions).toBeTruthy();
+    // /mydspace renders one of these per pool task, so a constant id duplicates across rows
+    expect(actions.nativeElement.id).not.toEqual('actions');
+    expect(actions.nativeElement.id).toContain(component.dso.id);
+  });
 });