Skip to content

Commit

Permalink
[ui] Show re-bound keyboard nav hints instead of their default values (
Browse files Browse the repository at this point in the history
…#20235)

* Rebinds show up as soon as you start rebinding

* Hint bind and rebind tests

* Orphaned getCommandByPattern method removed
  • Loading branch information
philrenaud committed Apr 18, 2024
1 parent 6f171e0 commit c407e62
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/20235.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: When you re-bind keyboard shortcuts they now correctly show up in shift-held hints
```
23 changes: 22 additions & 1 deletion ui/app/components/keyboard-shortcuts-modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,28 @@ export default class KeyboardShortcutsModalComponent extends Component {
@computed('keyboard.{keyCommands.length,displayHints}')
get hints() {
if (this.keyboard.displayHints) {
return this.keyboard.keyCommands.filter((c) => c.element);
let elementBoundKeyCommands = this.keyboard.keyCommands.filter(
(c) => c.element
);
// Some element-bound key commands have pairs can be re-bound by the user.
// For each of them, check to see if any other key command has its pattern
// as a defaultPattern. If so, use that key command's pattern instead.
let elementBoundKeyCommandsWithRebinds = [];
elementBoundKeyCommands.forEach((c) => {
let pair = this.keyboard.keyCommands.find(
(kc) =>
JSON.stringify(kc.defaultPattern) === JSON.stringify(c.pattern)
);
if (pair) {
elementBoundKeyCommandsWithRebinds.push({
...c,
pattern: pair.pattern,
});
} else {
elementBoundKeyCommandsWithRebinds.push(c);
}
});
return elementBoundKeyCommandsWithRebinds;
} else {
return [];
}
Expand Down
4 changes: 4 additions & 0 deletions ui/app/services/keyboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import localStorageProperty from 'nomad-ui/utils/properties/local-storage';
* @property {boolean} [custom]
* @property {boolean} [exclusive]
* @property {HTMLElement} [element]
* @property {string[]} [defaultPattern]
*/

const DEBOUNCE_MS = 750;
Expand Down Expand Up @@ -110,6 +111,7 @@ export default class KeyboardService extends Service {
{
label: 'Go to Variables',
action: () => this.router.transitionTo('variables'),
rebindable: true,
},
{
label: 'Go to Servers',
Expand Down Expand Up @@ -181,6 +183,7 @@ export default class KeyboardService extends Service {
if (persistedValue) {
set(command, 'pattern', JSON.parse(persistedValue));
set(command, 'custom', true);
set(command, 'defaultPattern', this.defaultPatterns[command.label]);
} else {
set(command, 'pattern', this.defaultPatterns[command.label]);
}
Expand Down Expand Up @@ -364,6 +367,7 @@ export default class KeyboardService extends Service {
this.clearBuffer();
set(cmd, 'recording', true);
set(cmd, 'previousPattern', cmd.pattern);
set(cmd, 'defaultPattern', cmd.defaultPattern || cmd.pattern);
set(cmd, 'pattern', null);
};

Expand Down
24 changes: 24 additions & 0 deletions ui/tests/acceptance/keyboard-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ module('Acceptance | keyboard', function (hooks) {
'end up on the clients page after typing g c'
);

await triggerEvent('.page-layout', 'keydown', { key: 'Shift' });
assert.dom('[data-shortcut="g,c"]').exists('g c shortcut is shown');
await triggerEvent('.page-layout', 'keyup', { key: 'Shift' });

triggerEvent('.page-layout', 'keydown', { key: 'g' });
await triggerEvent('.page-layout', 'keydown', { key: 'j' });
assert.equal(
Expand Down Expand Up @@ -180,6 +184,16 @@ module('Acceptance | keyboard', function (hooks) {
'text unchanged when I hit escape during recording'
);

// when holding shift, the previous "g c" command is now "r o f l"
await triggerEvent('.page-layout', 'keydown', { key: 'Shift' });
assert
.dom('[data-shortcut="g,c"]')
.doesNotExist('g c shortcut is no longer shown');
assert
.dom('[data-shortcut="r,o,f,l"]')
.exists('r o f l shortcut is shown in its place');
await triggerEvent('.page-layout', 'keyup', { key: 'Shift' });

await click(
'[data-test-command-label="Go to Clients"] button.reset-to-default'
);
Expand All @@ -188,6 +202,16 @@ module('Acceptance | keyboard', function (hooks) {
'[data-test-command-label="Go to Clients"] button[data-test-rebinder]'
)
.hasText('g c', 'Resetting to default rebinds the shortcut');

// when holding shift, the now-reset command is back to "g c"
await triggerEvent('.page-layout', 'keydown', { key: 'Shift' });
assert
.dom('[data-shortcut="g,c"]')
.exists('g c shortcut is back after reset to default');
assert
.dom('[data-shortcut="r,o,f,l"]')
.doesNotExist('r o f l shortcut is gone after reset to default');
await triggerEvent('.page-layout', 'keyup', { key: 'Shift' });
});

test('Rebound shortcuts persist from localStorage', async function (assert) {
Expand Down

0 comments on commit c407e62

Please sign in to comment.