Make all rows on gr-change-list tabbable

Previously the screenreader readout was done by removing all rows except
the selected row from the tab flow by setting tabindex = -1, but this
broke tabbing through the other non-selected rows.

This method was not breaking tab flow in the gr-file-list because the
rows are not using shadow DOMs. see
WICG/webcomponents#774

This change restores all rows to the tab flow by always setting tabindex
to 0, and maintains the screenreader readout by focusing the row when it
gets selected. Verified with VoiceOver on Mac OS.

Release-Notes: skip
Google-Bug-Id: b/235477335
Change-Id: I48ef40f18ce061e50ef71bd4df0cf311a25e683b

polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts

@@ -33,7 +33,7 @@ import {
 import {hasOwnProperty, assertIsDefined} from '../../../utils/common-util';
 import {changeListStyles} from '../../../styles/gr-change-list-styles';
 import {sharedStyles} from '../../../styles/shared-styles';
-import {LitElement, css, html} from 'lit';
+import {LitElement, css, html, PropertyValues} from 'lit';
 import {customElement, property, state} from 'lit/decorators';
 import {submitRequirementsStyles} from '../../../styles/gr-submit-requirements-styles';
 import {ifDefined} from 'lit/directives/if-defined';
@@ -69,9 +69,7 @@ declare global {
     'gr-change-list-item': GrChangeListItem;
   }
 }
-/**
- * @attr {Boolean} selected - change list item is selected by cursor
- */
+
 @customElement('gr-change-list-item')
 export class GrChangeListItem extends LitElement {
   /** The logged-in user's account, or null if no user is logged in. */
@@ -103,6 +101,8 @@ export class GrChangeListItem extends LitElement {
   @property({type: String})
   usp?: string;
 
+  @property({type: Boolean, reflect: true}) selected = false;
+
   // private but used in tests
   @property({type: Boolean, reflect: true}) checked = false;
 
@@ -137,6 +137,14 @@ export class GrChangeListItem extends LitElement {
       });
   }
 
+  override willUpdate(changedProperties: PropertyValues<this>) {
+    // When the cursor selects this item, give it focus so that the item is read
+    // out by screen readers and lets users start tabbing through the item
+    if (this.selected && !changedProperties.get('selected')) {
+      this.focus();
+    }
+  }
+
   static override get styles() {
     return [
       changeListStyles,

polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts

@@ -317,18 +317,17 @@ export class GrChangeListSection extends LitElement {
   ) {
     const ariaLabel = this.computeAriaLabel(change);
     const selected = this.computeItemSelected(index);
-    const tabIndex = this.computeTabIndex(index);
     return html`
       <gr-change-list-item
+        tabindex="0"
         .account=${this.account}
-        ?selected=${selected}
+        .selected=${selected}
         .change=${change}
         .config=${this.config}
         .sectionName=${this.changeSection.name}
         .visibleChangeTableColumns=${columns}
         .showNumber=${this.showNumber}
         ?showStar=${this.showStar}
-        tabindex=${tabIndex}
         .usp=${this.usp}
         .labelNames=${this.labelNames}
         aria-label=${ariaLabel}
@@ -375,11 +374,6 @@ export class GrChangeListSection extends LitElement {
     return index === this.selectedIndex;
   }
 
-  private computeTabIndex(index: number) {
-    if (this.isCursorMoving) return 0;
-    return this.computeItemSelected(index) ? 0 : -1;
-  }
-
   // private but used in test
   computeColspan(cols: string[]) {
     if (!cols || !this.labelNames) return 1;

polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts

@@ -231,18 +231,18 @@ suite('gr-change-list basic tests', () => {
     );
     assert.equal(elementItems.length, 3);
 
-    assert.isTrue(elementItems[0].hasAttribute('selected'));
+    assert.isTrue(elementItems[0].selected);
     await element.updateComplete;
     pressKey(element, 'j');
     await element.updateComplete;
     await section.updateComplete;
 
     assert.equal(element.selectedIndex, 1);
-    assert.isTrue(elementItems[1].hasAttribute('selected'));
+    assert.isTrue(elementItems[1].selected);
     pressKey(element, 'j');
     await element.updateComplete;
     assert.equal(element.selectedIndex, 2);
-    assert.isTrue(elementItems[2].hasAttribute('selected'));
+    assert.isTrue(elementItems[2].selected);
 
     const navStub = sinon.stub(GerritNav, 'navigateToChange');
     assert.equal(element.selectedIndex, 2);
@@ -319,7 +319,7 @@ suite('gr-change-list basic tests', () => {
     );
     assert.equal(elementItems.length, 3);
 
-    assert.isTrue(elementItems[0].hasAttribute('selected'));
+    assert.isTrue(elementItems[0].selected);
     await element.updateComplete;
 
     pressKey(element, 'x');
@@ -335,7 +335,7 @@ suite('gr-change-list basic tests', () => {
     await element.updateComplete;
 
     assert.equal(element.selectedIndex, 1);
-    assert.isTrue(elementItems[1].hasAttribute('selected'));
+    assert.isTrue(elementItems[1].selected);
 
     pressKey(element, 'x');
     await element.updateComplete;
@@ -349,7 +349,7 @@ suite('gr-change-list basic tests', () => {
     pressKey(element, 'j');
     await element.updateComplete;
     assert.equal(element.selectedIndex, 2);
-    assert.isTrue(elementItems[2].hasAttribute('selected'));
+    assert.isTrue(elementItems[2].selected);
 
     pressKey(element, 'x');
     await element.updateComplete;