Dynamic numeric keynav for multiple tables (#13482)

* Multiple tables init

* URL-bind enumerable keyboard commands and add to more taskRow and allocationRows

* Type safety and lint fixes

* Consolidated push to keyCommands

* Default value when removing keyCommands

* Remove the URL-based removal method and perform a recompute on any add

ui/app/components/keyboard-shortcuts-modal.js

@@ -1,6 +1,5 @@
 import Component from '@glimmer/component';
 import { inject as service } from '@ember/service';
-import { htmlSafe } from '@ember/template';
 import { computed } from '@ember/object';
 import Tether from 'tether';
 
@@ -28,7 +27,7 @@ export default class KeyboardShortcutsModalComponent extends Component {
    * hints: filter keyCommands to those that have an element property,
    * and then compute a position on screen to place the hint.
    */
-  @computed('keyboard.keyCommands.length', 'keyboard.displayHints')
+  @computed('keyboard.{keyCommands.length,displayHints}')
   get hints() {
     if (this.keyboard.displayHints) {
       return this.keyboard.keyCommands.filter((c) => c.element);
@@ -47,7 +46,7 @@ export default class KeyboardShortcutsModalComponent extends Component {
     });
     hint.binder = binder;
   }
-  untetherFromElement(self, _, { element, hint }) {
+  untetherFromElement(self, _, { hint }) {
     hint.binder.destroy();
   }
 }

ui/app/controllers/application.js

@@ -9,6 +9,7 @@ import codesForError from '../utils/codes-for-error';
 import NoLeaderError from '../utils/no-leader-error';
 import OTTExchangeError from '../utils/ott-exchange-error';
 import classic from 'ember-classic-decorator';
+// eslint-disable-next-line no-unused-vars
 import KeyboardService from '../services/keyboard';
 @classic
 export default class ApplicationController extends Controller {
@@ -22,6 +23,7 @@ export default class ApplicationController extends Controller {
    */
   @service keyboard;
 
+  // eslint-disable-next-line ember/classic-decorator-hooks
   constructor() {
     super(...arguments);
     this.keyboard.listenForKeypress();

ui/app/helpers/keyboard-commands.js

@@ -10,12 +10,10 @@ export default class keyboardCommands extends Helper {
   @service keyboard;
 
   constructor() {
-    console.log('kc const', ...arguments);
     super(...arguments);
   }
 
   compute([commands]) {
-    console.log('computing', commands);
     if (commands) {
       this.commands = commands;
       this.keyboard.addCommands(commands);

ui/app/modifiers/keyboard-shortcut.js

@@ -1,46 +1,33 @@
 import { inject as service } from '@ember/service';
 import Modifier from 'ember-modifier';
 import { registerDestructor } from '@ember/destroyable';
-import { assert } from '@ember/debug';
 
 export default class KeyboardShortcutModifier extends Modifier {
   @service keyboard;
+  @service router;
 
-  /**
-   * For Dynamic/iterative keyboard shortcuts, our patterns look like "Shift+0" by default
-   * Do a couple things to make them more human-friendly:
-   * 1. Make them 1-based, instead of 0-based
-   * 2. Prefix numbers 1-9 with "0" to make it so "Shift+10" doesn't trigger "Shift+1" then "0", etc.
-   * ^--- stops being a good solution with 100+ row lists/tables, but a better UX than waiting for shift key-up otherwise
-   *
-   * @param {string[]} pattern
-   */
-  cleanPattern(pattern) {
-    let patternNumber = pattern.length === 1 && pattern[0].match(/\d+/g);
-    if (!patternNumber) {
-      return pattern;
-    } else {
-      patternNumber = +patternNumber[0]; // regex'd string[0] to num
-      patternNumber = patternNumber + 1; // first item should be Shift+1, not Shift+0
-      assert(
-        'Dynamic keyboard shortcuts only work up to 99 digits',
-        patternNumber < 100
-      );
-      pattern = [`Shift+${('0' + patternNumber).slice(-2)}`]; // Shift+01, not Shift+1
+  modify(
+    element,
+    _positional,
+    {
+      label,
+      pattern = '',
+      action = () => {},
+      menuLevel = false,
+      enumerated = false,
     }
-    return pattern;
-  }
-
-  modify(element, [eventName], { label, pattern, action, menuLevel = false }) {
+  ) {
     let commands = [
       {
         label,
         action,
-        pattern: this.cleanPattern(pattern),
+        pattern,
         element,
         menuLevel,
+        enumerated,
       },
     ];
+
     this.keyboard.addCommands(commands);
     registerDestructor(this, () => {
       this.keyboard.removeCommands(commands);

ui/app/services/keyboard.js

@@ -1,13 +1,18 @@
+// @ts-check
 import Service from '@ember/service';
 import { inject as service } from '@ember/service';
 import { timeout, restartableTask } from 'ember-concurrency';
 import { tracked } from '@glimmer/tracking';
 import { compare } from '@ember/utils';
 import { A } from '@ember/array';
+// eslint-disable-next-line no-unused-vars
 import EmberRouter from '@ember/routing/router';
 import { schedule } from '@ember/runloop';
 import { action } from '@ember/object';
 import { guidFor } from '@ember/object/internals';
+import { assert } from '@ember/debug';
+// eslint-disable-next-line no-unused-vars
+import MutableArray from '@ember/array/mutable';
 
 const DEBOUNCE_MS = 750;
 
@@ -37,7 +42,10 @@ export default class KeyboardService extends Service {
   @tracked buffer = A([]);
   @tracked displayHints = false;
 
-  keyCommands = [
+  /**
+   * @type {MutableArray<Object>}
+   */
+  keyCommands = A([
     {
       label: 'Go to Jobs',
       pattern: ['g', 'j'],
@@ -144,26 +152,45 @@ export default class KeyboardService extends Service {
         console.log('Extra Lives +30');
       },
     },
-  ];
+  ]);
 
-  addCommands(commands) {
-    // Filter out those commands that don't have a label (they're only being added for at-a-glance hinting/highlights)
-    this.keyCommands.pushObjects(commands);
+  /**
+   * For Dynamic/iterative keyboard shortcuts, we want to do a couple things to make them more human-friendly:
+   * 1. Make them 1-based, instead of 0-based
+   * 2. Prefix numbers 1-9 with "0" to make it so "Shift+10" doesn't trigger "Shift+1" then "0", etc.
+   * ^--- stops being a good solution with 100+ row lists/tables, but a better UX than waiting for shift key-up otherwise
+   *
+   * @param {number} iter
+   * @returns {string[]}
+   */
+  cleanPattern(iter) {
+    iter = iter + 1; // first item should be Shift+1, not Shift+0
+    assert('Dynamic keyboard shortcuts only work up to 99 digits', iter < 100);
+    return [`Shift+${('0' + iter).slice(-2)}`]; // Shift+01, not Shift+1
   }
 
-  removeCommands(commands) {
-    this.keyCommands.removeObjects(commands);
+  recomputeEnumeratedCommands() {
+    this.keyCommands.filterBy('enumerated').forEach((command, iter) => {
+      command.pattern = this.cleanPattern(iter);
+    });
   }
 
-  @action
-  generateIteratorShortcut(element, [action, iter]) {
-    this.keyCommands.pushObject({
-      label: `Hit up item ${iter}`,
-      pattern: [`Shift+${iter}`],
-      action,
+  addCommands(commands) {
+    schedule('afterRender', () => {
+      commands.forEach((command) => {
+        this.keyCommands.pushObject(command);
+        if (command.enumerated) {
+          // Recompute enumerated numbers to handle things like sort
+          this.recomputeEnumeratedCommands();
+        }
+      });
     });
   }
 
+  removeCommands(commands = A([])) {
+    this.keyCommands.removeObjects(commands);
+  }
+
   //#region Nav Traversal
 
   subnavLinks = [];

ui/app/templates/allocations/allocation/index.hbs

@@ -184,6 +184,10 @@
           </t.head>
           <t.body as |row|>
             <TaskRow
+              {{keyboard-shortcut 
+                enumerated=true
+                action=(action "taskClick" row.model.allocation row.model)
+              }}
               @data-test-task-row={{row.model.name}}
               @task={{row.model}}
               @onClick={{action "taskClick" row.model.allocation row.model}}

ui/app/templates/clients/client/index.hbs

@@ -546,6 +546,10 @@
             </t.head>
             <t.body as |row|>
               <AllocationRow
+                {{keyboard-shortcut 
+                  enumerated=true
+                  action=(action "gotoAllocation" row.model)
+                }}
                 @allocation={{row.model}}
                 @context="node"
                 @onClick={{action "gotoAllocation" row.model}}

ui/app/templates/clients/index.hbs

@@ -81,13 +81,13 @@
             <th># Volumes</th>
             <th># Allocs</th>
           </t.head>
-          <t.body as |row iter|>
+          <t.body as |row|>
             <ClientNodeRow
               data-test-client-node-row
               @node={{row.model}}
               @onClick={{action "gotoNode" row.model}}
               {{keyboard-shortcut 
-                pattern=(array (concat "Shift+" iter))
+                enumerated=true
                 action=(action "gotoNode" row.model)
               }}
             />

ui/app/templates/components/job-page/parts/children.hbs

@@ -65,8 +65,8 @@
             Summary
           </th>
         </t.head>
-        <t.body @key="model.id" as |row iter|>
-          <JobRow data-test-job-row @job={{row.model}} @context="child" @iter={{iter}} />
+        <t.body @key="model.id" as |row|>
+          <JobRow data-test-job-row @job={{row.model}} @context="child" />
         </t.body>
       </ListTable>
       <div class="table-foot">

ui/app/templates/components/job-page/parts/recent-allocations.hbs

@@ -52,6 +52,10 @@
             @allocation={{row.model}}
             @context="job"
             @onClick={{action "gotoAllocation" row.model}}
+            {{keyboard-shortcut
+              enumerated=true
+              action=(action "gotoAllocation" row.model)
+            }}
           />
         </t.body>
       </ListTable>

ui/app/templates/components/job-page/parts/task-groups.hbs

@@ -36,6 +36,10 @@
           @data-test-task-group={{row.model.name}}
           @taskGroup={{row.model}}
           @onClick={{fn this.gotoTaskGroup row.model}}
+          {{keyboard-shortcut 
+            enumerated=true
+            action=(fn this.gotoTaskGroup row.model)
+          }}
         />
       </t.body>
     </ListTable>

ui/app/templates/components/job-row.hbs

@@ -1,6 +1,6 @@
 <td data-test-job-name
   {{keyboard-shortcut 
-    pattern=(array (concat "Shift+" @iter))
+    enumerated=true
     action=(action "gotoJob" @job)
   }}
 >

ui/app/templates/components/server-agent-row.hbs

@@ -1,6 +1,6 @@
 <td data-test-server-name
   {{keyboard-shortcut 
-    pattern=(array (concat "Shift+" @iter))
+    enumerated=true
     action=(action this.goToAgent)
   }}
 ><LinkTo @route="servers.server" @model={{this.agent.id}} class="is-primary">{{this.agent.name}}</LinkTo></td>

ui/app/templates/csi/volumes/index.hbs

@@ -65,15 +65,15 @@
             # Allocs
           </th>
         </t.head>
-        <t.body @key="model.name" as |row iter|>
+        <t.body @key="model.name" as |row|>
           <tr
             class="is-interactive"
             data-test-volume-row
             {{on "click" (action "gotoVolume" row.model)}}
           >
             <td data-test-volume-name
               {{keyboard-shortcut 
-                pattern=(array (concat "Shift+" iter))
+                enumerated=true
                 action=(action "gotoVolume" row.model)
               }}
             >

ui/app/templates/csi/volumes/volume.hbs

@@ -52,6 +52,10 @@
           </t.head>
           <t.body as |row|>
             <AllocationRow
+              {{keyboard-shortcut 
+                enumerated=true
+                action=(action "gotoAllocation" row.model)
+              }}
               @data-test-write-allocation={{row.model.id}}
               @allocation={{row.model}}
               @context="volume"
@@ -90,6 +94,10 @@
           </t.head>
           <t.body as |row|>
             <AllocationRow
+              {{keyboard-shortcut 
+                enumerated=true
+                action=(action "gotoAllocation" row.model)
+              }}
               @data-test-read-allocation={{row.model.id}}
               @allocation={{row.model}}
               @context="volume"

ui/app/templates/evaluations/index.hbs

@@ -69,7 +69,7 @@
             Placement Failures
           </th>
         </t.head>
-        <t.body as |row iter|>
+        <t.body as |row|>
           <tr
             data-test-evaluation="{{row.model.shortId}}"
             style="cursor: pointer;"
@@ -80,7 +80,7 @@
           >
             <td data-test-id
                 {{keyboard-shortcut 
-                  pattern=(array (concat "Shift+" iter))
+                  enumerated=true
                   action=(fn this.handleEvaluationClick row.model)
                 }}
             >

ui/app/templates/jobs/index.hbs

@@ -142,8 +142,8 @@
             Summary
           </th>
         </t.head>
-        <t.body @key="model.id" as |row iter|>
-          <JobRow @data-test-job-row={{row.model.plainId}} @job={{row.model}} @iter={{iter}} />
+        <t.body @key="model.id" as |row|>
+          <JobRow @data-test-job-row={{row.model.plainId}} @job={{row.model}} />
         </t.body>
       </ListTable>
       <div class="table-foot">

ui/app/templates/jobs/job/allocations.hbs

@@ -62,6 +62,10 @@
           </t.head>
           <t.body as |row|>
             <AllocationRow
+              {{keyboard-shortcut 
+                enumerated=true
+                action=(action "gotoAllocation" row.model)
+              }}
               @data-test-allocation={{row.model.id}}
               @allocation={{row.model}}
               @context="job"

ui/app/templates/jobs/job/task-group.hbs

@@ -197,6 +197,10 @@
             </t.head>
             <t.body @key="model.id" as |row|>
               <AllocationRow
+                {{keyboard-shortcut 
+                  enumerated=true
+                  action=(action "gotoAllocation" row.model)
+                }}
                 @data-test-allocation={{row.model.id}}
                 @allocation={{row.model}}
                 @context="taskGroup"

ui/app/templates/servers/index.hbs

@@ -21,8 +21,8 @@
           <t.sort-by @prop="datacenter">Datacenter</t.sort-by>
           <t.sort-by @prop="version">Version</t.sort-by>
         </t.head>
-        <t.body as |row iter|>
-          <ServerAgentRow data-test-server-agent-row @agent={{row.model}} @iter={{iter}} />
+        <t.body as |row|>
+          <ServerAgentRow data-test-server-agent-row @agent={{row.model}} />
         </t.body>
       </ListTable>
       <div class="table-foot">