Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Keyboard Navigation acceptance tests #13893

Merged
merged 6 commits into from
Jul 22, 2022
Merged

Keyboard Navigation acceptance tests #13893

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6dd0466
Acceptance tests for keyboard modal
philrenaud Jul 22, 2022
528f8b4
a11y audit fix and localStorage clear
philrenaud Jul 22, 2022
844fa52
Bind/rebind/localStorage tests
philrenaud Jul 22, 2022
547e0e9
Keyboard tests for dynamic nav and tables
philrenaud Jul 22, 2022
28d8d25
Rebinder and assert expectation
philrenaud Jul 22, 2022
6ae8f9a
Second percy snapshot showing hints no longer relevant
philrenaud Jul 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions ui/app/components/keyboard-shortcuts-modal.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
class="button is-borderless dismiss"
type="button"
{{on "click" (toggle "keyboard.shortcutsVisible" this)}}
aria-label="Dismiss"
>
{{x-icon "cancel"}}
</button>
Expand All @@ -19,7 +20,7 @@
</header>
<ul class="commands-list">
{{#each this.commands as |command|}}
<li>
<li data-test-command-label={{command.label}}>
<strong>{{command.label}}</strong>
<span class="keys">
{{#if command.recording}}
Expand All @@ -30,7 +31,7 @@
{{/if}}
{{/if}}

<button disabled={{or (not command.rebindable) command.recording}} type="button" {{on "click" (action this.keyboard.rebindCommand command)}}>
<button class="re-bind" disabled={{or (not command.rebindable) command.recording}} type="button" {{on "click" (action this.keyboard.rebindCommand command)}}>
philrenaud marked this conversation as resolved.
Show resolved Hide resolved
{{#each command.pattern as |key|}}
<span>{{clean-keycommand key}}</span>
{{/each}}
Expand All @@ -42,7 +43,7 @@
<footer>
<strong>Keyboard shortcuts {{#if this.keyboard.enabled}}enabled{{else}}disabled{{/if}}</strong>
<Toggle
data-test-cpu-toggle
data-test-enable-shortcuts-toggle
@isActive={{this.keyboard.enabled}}
@onToggle={{this.toggleListener}}
title="{{if this.keyboard.enabled "enable" "disable"}} keyboard shortcuts"
Expand All @@ -54,8 +55,9 @@
{{#if (and this.keyboard.enabled this.keyboard.displayHints)}}
{{#each this.hints as |hint|}}
<span
{{did-insert this.tetherToElement element=hint.element hint=hint}}
{{will-destroy this.untetherFromElement element=hint.element hint=hint}}
{{did-insert (fn this.tetherToElement hint.element hint)}}
{{will-destroy (fn this.untetherFromElement hint)}}
data-test-keyboard-hint
data-shortcut={{hint.pattern}}
class="{{if hint.menuLevel "menu-level"}}"
aria-hidden="true"
Expand Down
30 changes: 19 additions & 11 deletions ui/app/components/keyboard-shortcuts-modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Tether from 'tether';

export default class KeyboardShortcutsModalComponent extends Component {
@service keyboard;
@service config;

escapeCommand = {
label: 'Hide Keyboard Shortcuts',
Expand Down Expand Up @@ -42,18 +43,25 @@ export default class KeyboardShortcutsModalComponent extends Component {
}
}

tetherToElement(self, _, { element, hint }) {
let binder = new Tether({
element: self,
target: element,
attachment: 'top left',
targetAttachment: 'top left',
targetModifier: 'visible',
});
hint.binder = binder;
@action
tetherToElement(element, hint, self) {
if (!this.config.isTest) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried very hard to avoid using a config.isTest call, but Tether doesn't play nice with Ember tests at all. A simple check here solves a world of headaches.

let binder = new Tether({
element: self,
target: element,
attachment: 'top left',
targetAttachment: 'top left',
targetModifier: 'visible',
});
hint.binder = binder;
}
}
untetherFromElement(self, _, { hint }) {
hint.binder.destroy();

@action
untetherFromElement(hint) {
if (!this.config.isTest) {
hint.binder.destroy();
}
}

@action toggleListener() {
Expand Down
Loading