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.

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

Event type #7

Closed
ultimatedelman opened this issue Apr 30, 2013 · 9 comments
Closed

Event type #7

ultimatedelman opened this issue Apr 30, 2013 · 9 comments

Comments

@ultimatedelman
Copy link

When wiring up events, the action triggering the event would be awesome to have in the event object:

{
    slide: [some slide]
    , index: [number]
    , type: [keypress(left|right|spacebar)|slide|prev|next|whatever]
}

As it is, it's very hard to wire up things like history state when you can't discern between a popstate and a keypress. Maybe the option to pass in an additional param to the slide() method?

 deck.slide(index, myEventParams);

I know you're trying to keep byte size down, but I feel like the utility introduced by this would make this library exponentially more powerful. If nothing else, the ability to add custom values to the event object would be a huge improvement.

@ultimatedelman
Copy link
Author

Sorry for the craziness. This is actually still an issue.

@markdalgleish
Copy link
Member

I think your second idea is the best tradeoff of utility and size. Keeping it under 1KB could be tricky, but I can see that this would be a worthwhile change to core.

This is something I think I'll have to implement, since it touches everything- core APIs, tests, docs, and potentially other code will need to be optimised to fit in the 1KB limit.

@ultimatedelman
Copy link
Author

Awesome. I think you'll see my implementation in my pull request is pretty minimal and provides flexible functionality required by someone trying to discern by event type

@markdalgleish
Copy link
Member

Yeah, thanks for the pull request. It looks to be following your first suggestion, but I think allowing custom event payloads would solve your problem in a much more minimal and flexible way, since it would allow you to provide metadata around calls to next, prev and slide.

I've begun work on this already, and so far it's looking good.

@ultimatedelman
Copy link
Author

Awesome! Glad I could help :)

@markdalgleish
Copy link
Member

I just pushed v0.2.0, which supports custom event data.

You can see an example of it in the readme: https://github.com/markdalgleish/bespoke.js#custom-event-data

Thanks for the feedback, I think this is a really useful feature :)

@ultimatedelman
Copy link
Author

w00t!

I see the example for .next() and assume it works that way for .prev() too. What's the method signature for slide, since that takes a number for the slide index? Is it .slide(index, eventOpts)?

EDIT: a quick glance at the code shows me that this is, in fact, how you're handling it. Sweet!

@markdalgleish
Copy link
Member

You can see the full deck API reference here: https://github.com/markdalgleish/bespoke.js#deck-instance-properties :)

@ultimatedelman
Copy link
Author

yep, RTFM. oops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants