cyclejs-community / cycle-keyboard

A keyboard driver for cycle.js
MIT License
9 stars 2 forks source link

Ponder the most suitable API #23

Closed artfuldev closed 8 years ago

artfuldev commented 8 years ago

From our gitter conversation:

Sudarsan Balaji @artfuldev 14:53

@Widdershin I initially thought that keyboard.keyUp$.filter(ev => ev.displayKey == 'Ctrl') was a better API (because it conveys intent) than keyboard.keyUp('Ctrl'). Is that at fault?

Nick Johnstone @Widdershin 14:54

do all browsers support displayKey?

Sudarsan Balaji @artfuldev 14:54

It's a property that the driver adds

Nick Johnstone @Widdershin 14:54

ohhh interesting I think I would still prefer the latter because it's my most common use case but I can see the value of your style

Sudarsan Balaji @artfuldev 14:56

@Widdershin I would like to see which people prefer. We could obviously provide both, but it will make the API clobbered with too many functions offering the same. OTOH some APIs provide shortcut functions exposing the same functionality, so it may be acceptable. We should perhaps open up a discussion? Or are you extremely certain that the latter is the way to go?

Raquel Moss @raquelxmoss 14:58

@artfuldev I reckon we should discuss API (and/or copy some of this conversation over to) on Github. It might be nice to have some history on the repo that covers the why of decision making around the API

Sudarsan Balaji @artfuldev 14:58

@raquelxmoss good call.

artfuldev commented 8 years ago

I have used the keyCodes => displayKey converter from https://github.com/wesbos/keycodes and displayChar for keypress events I get using the method mentioned at http://javascript.info/tutorial/keyboard-events.

The javascript.info page I mentioned above also helped me provide the capsLock$ in the driver.

Little things like this are what I think we can provide with the driver as additional.

artfuldev commented 8 years ago

We can instead make use of the keycodes npm package that @raquelxmoss has used in cycle-keys.

artfuldev commented 8 years ago

The idea is to also provide an easy and hassle-free migration from cycle-keys, while keeping in mind the strengths and merits of the API we will be taking up.

artfuldev commented 8 years ago

@raquelxmoss @Widdershin can we get some things done? :)

Widdershin commented 8 years ago

I've been mulling it over and I think I still favor the method -> stream style over the new filter style.

The most common usecase by far will be an immediate filter. If we look at the the two most popular drivers for Cycle, it's clear there's a precedent for methods to events.

DOM.select('.foo').events('.click').elements() HTTP.select('github').flatten()

Both of these could be written with filters + maps, and yet I think the intent is more clear by having prebuilt methods in place.

Plus with the method style API users can still get a stream of any event by calling the method with no arguments.

We can still attach the displayKey property to the event so users can work with it in the stream itself.

raquelxmoss commented 8 years ago

I agree with @Widdershin, his suggestion might be a happy medium between the two approaches.

artfuldev commented 8 years ago

Ok, and in accordance with @raquelxmoss 's idea, we can also provide a keycode way for keyup and keydown... Let me start work on this.

artfuldev commented 8 years ago

Following this up in #27 Also see #28