rmariuzzo / checkboxes.js

☑️ A jQuery plugin that gives you nice powers over your checkboxes.
http://rmariuzzo.github.io/checkboxes.js
MIT License
134 stars 25 forks source link

Selecting the end of the range requires shift-clicking exactly the checkbox #7

Open andrewmy opened 10 years ago

andrewmy commented 10 years ago

At least in Firefox 32: shift-clicking the second checkbox label doesn't work. Works in Chrome though.

rmariuzzo commented 10 years ago

Hey @andrewmy! Thanks for letting me know about that issue, I have just tried to reproduce using Firefox 30 on Fedora 19, and everything works as expected. However I'm upgrading Firefox to get version 32 and will try to reproduce it.

Did you tried the range selection with the demo at: http://rmariuzzo.github.io/checkboxes.js/#range-selection-of-checkboxes ?

andrewmy commented 10 years ago

Hey, thanks for reply. I tried the demo too, same: the second click has to happen on the checkbox. If it's on the label, it just selects the text between the items. Observing it on Win 7, will look on a Mac later.

rmariuzzo commented 10 years ago

Thanks for providing more information, I will make sure to create some test cases when clicking on labels.

rmariuzzo commented 10 years ago

Investigating a little bit looks like this issue only occurs in Firefox. There's a bug reporter in their tracker: https://bugzilla.mozilla.org/show_bug.cgi?id=559506

Since that bug is present since version 3 of Firefox, I will think on a way to circumvent that issue in Firefox.

iajrz commented 9 years ago

Chrome 39 exhibits the same behavior.

rmariuzzo commented 9 years ago

We should address this issue ASAP.

ramikassab commented 9 years ago

@rmariuzzo I tried using your awesome plugin on a set of checkboxes in a div that have been turned into Bootstrap 3 buttons, which essentially hides the checkbox and makes the label look like a button with an inactive and active state like a checkbox would have. Unfortunately, at least in Chrome 39, I was unable to get range selection to work and I suspect that it may be due to this issue. Removing the data-toggle="buttons" from the containing div revealed the checkbox elements and, therefore, worked... but my buttons were messed up :(

Here's a screenshot of the Bootstrap 3 buttons I'm trying to use range selection on: image

Would love to see a fix for this!

rmariuzzo commented 9 years ago

@ramikassab thanks for the feedback and detailed information, I will check on this right now.

I'm on the chat: https://gitter.im/rmariuzzo/checkboxes.js

rmariuzzo commented 9 years ago

@ramikassab I have made a simple test case for your scenario. I can see the issue you described. However, no feature from this plugin work well with Twitter Boostrap 3 checkbox/radio buttons. The main reason is that Twitter Boostrap 3 checkbox/radio buttons implementation doesn't listen to checkbox/radio change's events. Thus, there's no somehow-standard way to tell Twitter Boostrap 3 checkbox/radio buttons that underlying inputs got its value changed.

For now, I'm assuming that particular issue should be workarounded with event listeners... but I'm still unable to figure a way out. If you will need help on this scenario, let me know to dig deeper.

johnpalaganas commented 9 years ago

@rmariuzzo Any updates on being able to do range select by clicking on the label instead? Thanks!

rmariuzzo commented 9 years ago

@johnpalaganas I will invest ~1hr now to come with a fix to this. I'm thinking to cover those cases:

  1. When the label wraps the checkbox.
<label>
    <input type="checkbox" /> Apple
</label>
  1. When the label is associated to the checkbox via for attribute.
<input type="checkbox" id="apple" />
<label for="apple">Apple</label>

Any suggestions will be highly appreciated. :smile:

johnpalaganas commented 9 years ago

@rmariuzzo If you can cover those two cases, it would be great! :)

rmariuzzo commented 9 years ago

Thanks for your quick reply @johnpalaganas! I will create a separate branch for this.

johnpalaganas commented 9 years ago

Well thanks @rmariuzzo for your proactive approach!

rmariuzzo commented 9 years ago

@johnpalaganas I have just added support to labels for range selection: https://github.com/rmariuzzo/checkboxes.js/tree/range-improvements

I have only tested it with Chrome 41, later today I will test in other browsers.

I have created added a quick test to play around, but somehow it doesn't behave as my local environment: http://plnkr.co/edit/iPIep0cWO5SAxGoTQ4Ng

I need to fix this.

johnpalaganas commented 9 years ago

@rmariuzzo Thanks! Not working on my end, I hope you do find a solution :)

rmariuzzo commented 9 years ago

I'm here again trying to find a definitive solution to this issue. It has been a long time.

m1dst commented 8 years ago

The version in the test branch seems to work for me where I hide the checkbox and style the label and use the FOR property to associate them. Just tested in FF42, Edge and CH47. Great job.

I caveat this with 2 small changes to make it suit my situation. I am toggling a class to TR when each checkbox is checked. Out of the box the plugin is not firing the change event of the checkboxes so I manually trigger it. Not sure if it is the best approach but it works for me.

// Toggle checked property of the related checkbox.
if ($checkbox && !$checkbox.is(':disabled')) {
      $checkbox.prop('checked', !$checkbox.prop('checked'));
      $checkbox.trigger("change");     // I added this line
}

$checkboxes.slice(start, end)
      .filter(':not(:disabled)')
      .prop('checked', $checkbox.prop('checked'))
      .trigger("change");         // I added this line too