JoshBarr / on-media-query

A neat way to trigger JS when media queries change. No jQuery required.
283 stars 33 forks source link

Simplify the queries passed into init #3

Closed cameronhunter closed 11 years ago

cameronhunter commented 12 years ago

Instead of using an array of objects to define the queries I've changed it use an object. The key of the object is the context (skinny, mobile etc) and the value can be either the callback function or an array of callback functions.

Internally all the callbacks are kept as an array against the context key - this means that addQuery just appends another callback onto the context.

I think it makes for a nicer API - but that's just my opinion!

cameronhunter commented 12 years ago

Sorry - didn't realise my editor had replaced the tabs with spaces. I can create another pull request with minimal changes if you want?

JoshBarr commented 12 years ago

Spaces are fine..probably better that way :) This is really nice. While you're there though... we just modified the addQuery function to trigger callbacks automatically if you're attaching them to the current context... ie, if you add a new desktop function while desktop is active, it'll fire it straight away. Saves having to do it manually. Do you want to roll this in before we pull it?

cameronhunter commented 12 years ago

Think that's it rolled in now :)

JoshBarr commented 12 years ago

Hey Cameron,

We were using the plugin on a project the other day and rolled in some new changes. One thing that's come up is the need to have the same function execute in a range of contexts. For us, the easiest way to do this was:

var query = {
             context: ['skinny', 'desktop'],
             call_in_each_context: false,
             callback: function() {
                    something_awesome();
             };
      }

The gives us the ability to have a function that executes once in either skinny or desktop, for instance. This is quite handy for things like video, where you don't really want to re-initialise if the user resizes their browser window a little.

Have you got any thoughts about how to marry this functionality with the object-key approach? I agree the API is nicer and less verbose, but it's also harder to extend with custom parameters, and obviously we can't have array as the key!

Potentially init() is a bit of a red herring, and we could move the whole API to a modified addQuery. This could accept the whole gamut of options and configurations.. multiple keys, multiple functions etc. What do you think?

Really appreciate your work on this!

mhulse commented 12 years ago

I thought I could fork this project and try to roll in Cameron's code into the latest master branch... Unfortunately, after having looked over the code, I'm not sure if I have the time. On top of not fully understanding all of the ins/outs of the code, there have be a few commits since this pull request and the code has changed enough to make things not as easy to update as I thought.

I would love to use the object-key approach myself... I'll keep dinking with the code, but just thought I would post a message here to see if anyone has any tips/feedback/ideas on how to incorporate this pull request with the latest master branch (I'd love to avoid re-inventing a wheel here).

Thanks!

Btw, this script has been a life saver! Thanks Josh! :)

mhulse commented 11 years ago

Let's go ahead and close this. The code has changed so much since this pull request was made.