Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-24 23:48:13 -05:00
Code Issues Activity

Prevent keymaster bindings from causing failed tests (#976)

Browse source 
no issue
- keymaster bindings could be kept around between tests with references to objects that should have been deleted. This was manifesting itself with a Tags acceptance test failing because one of the key binding tests triggered two actions, one on the route under test and another on a route that was kept around from a previous Error Handling acceptance test (previous test intentionally had an aborted transition so the action in the current test was complaining about the first transition not being complete)
- add guard to ensure that shortcuts can't be registered multiple times on an object
- remove all `default` scope keymaster bindings when destroying a test app instance
This commit is contained in:
Kevin Ansfield 2018-03-19 18:39:17 +00:00 • committed by GitHub
parent 2a55c5767f
commit 069bccdd94
2 changed files with 32 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
ghost/admin/app/mixins/shortcuts.js
View file

@ -44,27 +44,34 @@ key.setScope('default');
* Find out more at the keymaster docs * Find out more at the keymaster docs
*/ */
export default Mixin.create({ export default Mixin.create({
_hasRegisteredShortcuts: false,
registerShortcuts() { registerShortcuts() {
let shortcuts = this.get('shortcuts'); if (!this._hasRegisteredShortcuts) {
let shortcuts = this.get('shortcuts');
Object.keys(shortcuts).forEach((shortcut) => { Object.keys(shortcuts).forEach((shortcut) => {
let scope = shortcuts[shortcut].scope || 'default'; let scope = shortcuts[shortcut].scope || 'default';
let action = shortcuts[shortcut]; let action = shortcuts[shortcut];
let options; let options;
if (typeOf(action) !== 'string') { if (typeOf(action) !== 'string') {
options = action.options; options = action.options;
action = action.action; action = action.action;
} }
key(shortcut, scope, (event) => { key(shortcut, scope, (event) => {
// stop things like ctrl+s from actually opening a save dialogue // stop things like ctrl+s from actually opening a save dialogue
event.preventDefault(); event.preventDefault();
run(this, function () { run(this, function () {
this.send(action, options); this.send(action, options);
});
}); });
}); });
});
this._hasRegisteredShortcuts = true;
}
}, },
removeShortcuts() { removeShortcuts() {
@ -74,5 +81,10 @@ export default Mixin.create({
let scope = shortcuts[shortcut].scope || 'default'; let scope = shortcuts[shortcut].scope || 'default';
key.unbind(shortcut, scope); key.unbind(shortcut, scope);
}); });
},
willDestroy() {
this._super(...arguments);
this.removeShortcuts();
} }
}); });

5
ghost/admin/tests/helpers/destroy-app.js
View file

@ -1,3 +1,4 @@
/* global key */
import {run} from '@ember/runloop'; import {run} from '@ember/runloop';
export default function destroyApp(application) { export default function destroyApp(application) {
@ -6,5 +7,9 @@ export default function destroyApp(application) {
server.shutdown(); server.shutdown();
} }
// extra check to ensure we don't have references hanging around via key
// bindings on supposedly destroyed objects
key.deleteScope('default');
run(application, 'destroy'); run(application, 'destroy');
} }