Johann-S / bs-customizer

Create a custom Bootstrap 4.x build
https://johann-s.github.io/bs-customizer/
MIT License
17 stars 2 forks source link

toggle all elements #47

Closed Johann-S closed 6 years ago

Johann-S commented 6 years ago

Fixes #4

MartijnCuppens commented 6 years ago

Only the checkboxes of the first group are prefixed with .js-, shouldn't we do this with all elements?

Maybe we can also do something like:

<button class="btn btn-outline-secondary btn-sm float-right js-btn-toggle" data-target=".js-javascript-checkbox">
    Toggle all
</button>
and
<button class="btn btn-outline-secondary btn-sm float-right js-btn-toggle" data-target=".js-layout-checkbox">
    Toggle all
</button>
$('.js-btn-toggle').on('click', function (event) {
  event.preventDefault()
  const $targets = $($(this).data('target'));
  // ...
})

so that we don't need the if/else construction?

Johann-S commented 6 years ago

I did that because it was for the JS part (.js-checkbox because we are in the JavaScript plugin section)

And with your solution, that's right we don't have if/else but we query our elements on each click, instead of one. We don't have dynamic content so for me it's not a good practice to do that here

MartijnCuppens commented 6 years ago

@Johann-S, maybe there is a solution which includes the best parts of both solutions?

Johann-S commented 6 years ago

I don't know 🤔

I'll merge this PR like that and maybe we'll change that later if we have ideas