adopted-ember-addons/ember-keyboard

First time use of this addon - is this the best user interface?

optikalefx opened this issue · 9 comments

I mean absolutely no disrespect - I'm very grateful to everyone who has worked on this project. It is the most popular ember keyboard addon.

Being someone using this for the first time, the interface in which to use this is very verbose. And boilerplate.

I takes 3 imports per file

import { keyUp } from 'ember-keyboard';
import { on } from "@ember/object/evented";
import { EKMixin } from 'ember-keyboard';

I have to extend with the mixin in every file

export default Controller.extend(EKMixin, {

I have to initialize the addon in every file

init() {
	this._super(...arguments);
	this.set('keyboardActivated', true);
},

And then this unconventional syntax for usage

nextProduct: on(keyUp('KeyS'), function() {
	console.log('next');
}),

Is this the best interface for this? I just thought starting the discussion would be a good idea.

The keyUp and EKMixin can be merged, so one line less. The keyboardActivated bit is a bit tedious, that's true.

We should have a different mixin for that, like EKOnInsertMixin.

Regarding the "unconventional syntax": what do you find unconventional about that? All event listeners in Ember work that way.

You might also use the @on decorator from ember-decorators, which makes for quite sexy syntax IMHO:

@on(keyUp('ctrl+KeyS'))
save() {
  // ...
}

Good call on importing both items.
What circumstance would exist where you would not want to do the this.set('keyboardActivated', true); step?

The @on syntax would be good, but it doesn't seem to work.

import { on } from 'ember-decorators/object';

@on(keyUp('alt+KeyS'))
	toggleClientSku() {
		// stuff
	},

Throws ember-metal.js:3988 TypeError: (0 , _object.on) is not a function

My mistake, it's import { on } from 'ember-decorators/object/evented'
So you still need an additional import for that.

I guess my preferred interaction with this library would not be events, but actions. You would register your keys with an action name. And that action would get called. That makes the most ember sense to me. Maybe it's just the kinds of apps that I build, but I almost never use on for anything. I only have used it when an addon has me use it.

I know key events sounds like they should be events, but ember is very actiony, and I think the action style / syntax makes the most sense to me.

The way that ember-keyboard-shortcuts does this clicks for me. You mixin the thing. Then you define a pre-named keyboardEvents function. That function maps keys to action names. Then you just write your actions.

However, when doing this in practice, actually takes more boilerplate setup then your current method does. So maybe events is best. I'd have to spend more time thinking. Maybe I'll mock up an ideal usage so we can talk about something tangible.

So you still need an additional import for that.

Trying to circumvent a feature / import only for saving a LOC doesn't make a lot of sense to me. If you've ever written an app in Java or C++ you'll be no stranger to over 50 lines of only imports.

But when you're writing code in these languages, imports are usually automatically added / removed by the IDE. We don't have this in Ember yet, but I think, we're not so far away from that brighter future.

I guess my preferred interaction with this library would not be events, but actions. You would register your keys with an action name. And that action would get called. That makes the most ember sense to me.

Actions require you to wire them up in the template. So in reality you're not avoiding on, but shifting that responsibility to a different place.

Furthermore, how would you actually want to do the wiring then? By action name, like keyUpCtrlAltKeyS? Maybe it's just me, but that doesn't appeal to me at all.

Yea not anything pre-defined like that. It would be something like

keyboardEvents: [
{
     event: "KeyUp",
     key: "alt+KeyS",
     action: "someAction"
}
]

And then someAction in this controller or component would get fired. I don't think I like this more than the on method though. I think I like your event method better after writing both out.

The only thing really after going through some scenarios that feels out of place, is the init activation. If I'm pulling in that mixin, isn't almost always the case that we are activating the keyboard? I think your idea of making that part of the mixin would work well.

Thanks for getting to this, @buschtoens!

@optikalefx, if you can find a way to simplify the interface for ember-keyboard, that would totally warrant a 3.0. Please let us know anything you think of!

As for activating the keyboard, there are a few cases where you wouldn't want to do that on init or didInsertElement. For instance, let's say you have comment form and you want to submit on Enter, but only if the comment textfield is focused. In that case, you'd want to activate the keyboard on focus, but not on insert.

Currently, there are a couple of mixins that handle activation for components, namely EKOnFocusMixin and EKOnInsertMixin. They don't support controllers, though. If you wanted to set them up to do that, I think that would make an excellent incremental improvement that wouldn't require a major version release.

Thanks @patience-tema-baron I will look into creating a mixin for controllers. If I have a chance to look into an alternate interface, I'll post that for everyone to discuss.

I've submitted a PR for the onInit mixin #74

Closing this since it was somewhat addressed by #74. I think there is room for improvement and a need to develop and non-mixin based API. Ideas for this can be discussed in #115