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

First try at adding button support #64

Closed
andriijas opened this issue Jan 31, 2013 · 7 comments
Closed

First try at adding button support #64

andriijas opened this issue Jan 31, 2013 · 7 comments

Comments

@andriijas
Copy link
Contributor

Hi

I need help with this one, if you want button support that is :)

https://github.com/andriijas/backbone.stickit/tree/buttons

The test cases I added fail, which implies there is more work to do on my changes.

For test case 1 single buttons I dont know whats wrong - the options.bindKey != bindKey check fail

For test case 2 multiple buttons I think it has to do with #62, the value of the first button is used, not the clicked one.

Thanks

@delambo delambo mentioned this issue Feb 3, 2013
@andriijas
Copy link
Contributor Author

Hi @delambo

Tried to get my button groups going with the new handler support, first of all - genius done!

The stickit source code is much easier to grasp.

I have toyed with passing the event object to the getVal method, this way I can figure out which of the buttons that was actually clicked. This is partially related to #62 because the first $el is used, but console logging $el.length shows that it has all the correct buttons. I just cant figure out which one is the one being clicked without having access to the event object.

What do you think should that be figured out on a higher level or in handler level?

@delambo
Copy link
Member

delambo commented Feb 12, 2013

I like the idea of passing the event into getVal but that parameter will be inconsistent since getVal is used in updateViewBindEl at times when no event was triggered. That could be an ugly feature in the api.

You might also need to set some data into the dom, marking the current button/value to get the value when no event exists.

@andriijas
Copy link
Contributor Author

Yeah this was what I realized as well. Haven't come up with something useful to make a pull request of yet.

@delambo
Copy link
Member

delambo commented Feb 13, 2013

Something stinks about this - it shouldn't be hard to set this up. The only reason why getVal is used in updateViewBindEl is to pass the original value for a bound element into afterUpdate. That's a nice-to-have, but it's probably more beneficial to pass the event into getVal.

@andriijas
Copy link
Contributor Author

I guess this is a design decision you will have to make :)

Personally I have not used afterUpdate a lot yet, I just listen to change events on the model instead which are effectively triggered after the model has been updated.

@Listento @model, 'change:foo', @afterUpdateFooQuxBar etc

delambo added a commit that referenced this issue Feb 14, 2013
@delambo
Copy link
Member

delambo commented Feb 14, 2013

I think my last push to master should help. Check it out.

@andriijas
Copy link
Contributor Author

Here is my submission to the stickit handlers store :)

Thanks for all the help @delambo

Backbone.Stickit.addHandler([
  {
    selector: '.btn-group button',
    events: ['click'],
    update: function($el, val) {
      $el.filter("[value=" + val + "]").addClass('active');
    },
    getVal: function($el, event) {
      $el.removeClass('active');
      $(event.currentTarget).addClass('active').val();
    }
  }
]);

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

No branches or pull requests

2 participants