georgemandis / konami-js

Adding the Konami Code easter egg to your projects since 2009! Compatible with gestures on smartphones and tablets as well. Compatible with all front-end frameworks and vanilla JavaScript
http://konamijs.mand.is/
MIT License
959 stars 122 forks source link

Version 2 api changing #53

Closed DavideCarvalho closed 3 years ago

DavideCarvalho commented 5 years ago

Since the idea is to change the major version, do you think the api should stay the same? Or do you think it needs some changes?

georgemandis commented 5 years ago

I haven't thought about this much, but I'm definitely open to discussing changes.

Right now you can create instances of Konami-JS and pass either a string (which is will assume is a URL to redirect to) or a function, which will execute

const easterEgg1 = new Konami("https://example.com");
const easterEgg2 = new Konami(() => { console.log("Something!"); });

For backwards-compatibility I'd like to consider keeping this behavior, but add an option to pass JSON for configuration:

const easterEgg = new Konami({
    keyboardSequence:   "ABABAB",
    gestureSequence: ["UP", "UP", "DOWN", "DOWN"],
    events: {
        keyboard: () => { console.log ("This is the function that executes when the keyboard sequence is triggered"); },
        gesture: () => { console.log ("This is the function that executes when the gesture sequence is triggered"); },
    }
})

That schema is off the top of my head, but something like that. It would be nice to:

Those are my thoughts for now. I'd love to hear others!

What do you think? How would you change it?

georgemandis commented 5 years ago

@DavideCarvalho Sorry to leave this for a couple weeks. I should go through and prune the issues, but I decided to do a base rewrite exploring some of the ideas I raised here. If you look at the 2.0 branch you'll see the new code. Would love to hear feedback!

georgemandis commented 5 years ago

I wanted to elaborate on how I changed things. These two approaches still work, which should ensure backwards compatibility:

Passing a string

const easterEgg = new Konami("https://example.com");

When the Konami code is successfully entered (via keyboard, gesture or gamepad) it will redirect the browser to the website passed as a string.

Passing a function

const easterEgg = new Konami(() => { alert("Konami Code!"); })

When the Konami code is successfully entered (via keyboard, gesture or gamepad) it will execute the function passed.

Passing an object

If you want to change the sequence for either keyboard, gesture or gamepad you can pass an object:

const easterEgg = new Konami({
    keyboard: {
        sequence: ['KeyA', 'KeyB', 'KeyC'],
        event: () => { console.log('ABC!") }
      },
    gesture: {
        sequence: ['UP', 'UP', 'UP', 'UP'],
        event: () => { console.log('Gesture easter egg triggered!") }
    }
})

You will also need to pass something to the event property. You can pass a string (for URL redirect) or a function.

This feels like an okay way to keep the zero-config, copy-and-paste approach 90% of people use while making the customization approach feel a little more organized. I'm very open to feedback and other approaches!

DavideCarvalho commented 5 years ago

I like the idea of having both function or an object as a parameter, but I don't know if this should be on the constructor

// Or even break a little more konami .addKonamiKeyboardObject({ sequence: ['KeyA', 'KeyB', 'KeyC'], event: () => { console.log('ABC!') }, }) .addKonamiGestureObject({ sequence: ['UP', 'UP', 'UP', 'UP'], event: () => { console.log('Gesture easter egg triggered!') } })

- Or continue with new Konami, and export a fluent api
```javascript
const konami = require('konami');
const newEvent = new Konami();
newEvent
  .addGesture('UP', 'UP', 'UP', 'UP')
  .andDispatchEvent(() => { console.log('ABC!') })
  .addKeyboard('KeyA', 'KeyB', 'KeyC')
  .andDispatchEvent(() => { console.log('Gesture easter egg triggered!') })
  .run()

I think this way the api will be easier to use, easier to remember the comands and the comands will have a single responsability.

hktr92 commented 5 years ago

Or you could use factory instead... would provide simpler api, internal stacking and more... here's how i imagine the interface of Konami

interface Konami {
    sequence(...keys: string[])
    listener(fn: callable)
    enableGesture()
    enableGamepad()
}

a sample usage:

Konami('mine').sequence(Konami.UP, Konami.DOWN).listener(() => doStuff())
Konami('craft').sequence(Konami.C, Konami.K).listener(() => moreStuff())

// for gesture
Konami("swiper").enableGesture().sequence(Konami.UP).listener(() => console.log("detected UP swipe"))

// getting a Konami object
const k1 = Konami('mine')

// or enabling / disabling objects
Konami('mine').disable()

// so we can play with multiple sequences, something like:
const enableK2 = () => Konami('craft').enable()
Konami("mine").sequence(Konami.K, Konami.NUM_1).listener(enableK2)

I know using "new stuff()" in javascript is fancy and stuff, but to have methods like "addKeyboardSequence" and "addSwipeSequence" is overkill.

Konami can be used to detect following types of sequences:

Calling constants like Konami.KEY.UP, Konami.PAD.UP or Konami.SWIPE.UP would be nice, but you'd have to write three separate handlers for these three separate input devices. Why don't we use the same interface for all of them? Okay, mobile and gamepad doesn't have all the keys, and the gamepad has extra buttons non-existent on web. But we can work on an abstract interface that allows us to target specific devices without requiring to register same handler for three different targets.

if you opt for my idea, then the listener callback could be adjusted to have only one parameter, and that's the detected device that triggered the sequence, so you can do something like this:

const konamiCb = triggerType => {
    switch (trigger) {
        case "keyboard": console.debug("Triggered from keyboard");
        case "gamepad": console.debug("Triggered from gamepad");
        case "touch": console.debug("Triggered from touch device");
    }
}

Dunno, just an idea :)

georgemandis commented 5 years ago

Thanks for contributing ideas @hktr92! I appreciate it :)

Your approach is interesting. There are aspects I like! It does seem a little verbose. A basic implementation of the Konami Code would look something like this I think:

Konami('easteregg').sequence(Konami.UP, Konami.UP, Konami.DOWN, Konami.DOWN, Konami.LEFT, Konami.RIGHT, Konami.LEFT, Konami.RIGHT, Konami.B, Konami.A).listener(() => doStuff())

Does that sounds right?

An interesting thing that came up in another thread: what if you only wanted to listen on a particular element? Someone submitted an example of listening for gesture events and attaching them to specific elements in the page, which I thought was interesting.

Maybe the thing you want to attach the listener to could be included an an argument for listener() ?

hktr92 commented 5 years ago

@georgemandis it seems fine to me. Even it might be a bit verbose, it would be easier to know what the code does (a few of my colleagues told me that they didn't understood my code unless I made it a bit verbose, so i got used to it).

For attaching Konami to a given element, I guess it would be nice to have a method called target(elem: HTMLElement | string) (or attachTo?) which can accept an HTMLElement instance or a string for query selection (ES6 has native support for this and to have a full event object as first listener parameter, and the second one custom data.

Here's the full API that I would use if I'd start this project by my own:

/**
 * KonamiCollection is a helper for storing and managing Konami instances
 */
declare interface KonamiCollection {
    /**
     * Retrieves an existent Konami instance
     * 
     * @param name 
     */
    get(name: string): Konami;

    /**
     * Checks if a given Konami instance exists
     * 
     * @param name 
     */
    has(name: string): Konami;

    /**
     * Adds a new Konami instance to the internal stack
     * 
     * @param name 
     */
    add(name: string): Konami;
}

/**
 * The main Konami interface
 */
declare interface Konami {
    /**
     * @param keys An Array<string> of key sequence, which are interpreted in the given order.
     */
    sequence(...keys: string[]): Konami;

    /**
     * What element do we want to target?
     * 
     * By default, it is the document's body.
     * @param elem 
     */
    target(elem: HTMLElement|string): Konami;

    /**
     * Subscribe a callable to the sequence event.
     * 
     * The callable must have two params:
     *      - event: object The event object
     *      - data: object Custom data object
     * @param fn 
     */
    listener(fn: CallableFunction): Konami;

    /**
     * Enables support for gestures
     */
    enableGesture(): Konami;

    /**
     * Enables support for gamepad
     */
    enableGamepad(): Konami;

    /**
     * Enables the engine itself
     */
    enable(): Konami;

    /**
     * Disables the gesture feature
     */
    disableGesture(): Konami;

    /**
     * Disables the gamepad feature
     */
    disableGamepad(): Konami;

    /**
     * Disables the engine itself
     */
    disable(): Konami;
}

/**
 * Konami factory function
 * 
 * This function covers:
 * - Retrieves an existent Konami instance from KonamiCollection
 * - Creates and stores a new Konami instance inside KonamiCollection
 * 
 * Example of usage:
 * const my = Konami("body").sequence(Konami.UP).listener((evt, data) => console.debug("Code triggered: ", data.sequence.toString()))
 * const el = Konami("button").sequence(Konami.DOWN).target("#game-instance").listener((evt, data) => console.debug("Code triggered in element ", evt.delegateTarget.nodeName));
 * 
 * @param name 
 */
declare function Konami(name:string): Konami;