1
\$\begingroup\$

I'm using bootstrap buttons, and in this case I have 5 different buttons with 5 different classes. When one of the buttons is clicked on, I want it to be the only one filled in, and the others to only be the outline. Here is how I currently do it:

$(".canvas_btn").on("click",function(){
  for(var i = 0; i < $(".canvas_btn").length; i++){
    document.getElementById($(".canvas_btn")[i].id).className = document.getElementById($(".canvas_btn")[i].id).className.replace("btn-outline-","btn-");
    document.getElementById($(".canvas_btn")[i].id).className = document.getElementById($(".canvas_btn")[i].id).className.replace("btn-","btn-outline-");
  }
  document.getElementById(this.id).className = document.getElementById(this.id).className.replace("btn-outline","btn");   
});
<script src="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/js/bootstrap.min.js"></script>
<link href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" rel="stylesheet"/>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<button class="btn btn-outline-info 	  	canvas_btn" align="left" 	id="new_text_btn">Text</button>
<button class="btn btn-outline-secondary  canvas_btn" align="left" 	id="new_audio_btn">Audio</button>
<button class="btn btn-outline-success 		canvas_btn" align="left" 	id="new_image_btn">Image</button>
<button class="btn btn-outline-danger 		canvas_btn" align="left" 	id="new_video_btn">Video</button>
<button class="btn btn-outline-warning 		canvas_btn" align="left" 	id="new_input_btn">Input</button>

It feels a bit odd the way I've done it - is there a more elegant way to do it? A way to use jQuery to make it tidier would be great!

I'm also not sure how I'm causing the error in the console - I think it's to do with the bootstrap script I call in, so don't know what I can do about that.

\$\endgroup\$
  • 2
    \$\begingroup\$ Bootstrap has radio buttons that do what you're wanting. \$\endgroup\$ – esote Nov 25 '18 at 23:20
2
\$\begingroup\$

Here's how I refactored your code:

$(document).ready(function () {

    $(".canvas_btn").on("click", function () {

        var idOfClickedElem = $(this).attr('id');
        $.each($('.canvas_btn'), function () {
            if (idOfClickedElem === $(this).attr('id')) {
                $(this).attr('class', getFilledClassName($(this).attr('class')))
            }
            else {
                $(this).attr('class', getOutlineClassName($(this).attr('class')))
            }
        });

    });

    function getOutlineClassName(currentClasses) {
        return currentClasses.replace("btn-outline-", "btn-").replace("btn-", "btn-outline-")
    }

    function getFilledClassName(currentClasses) {
        return currentClasses.replace("btn-outline", "btn")
    }


});

Instead of mixing jQuery and regular JavaScript selectors, I just used jQuery. I'm not a huge fan of the contents of getOutlineClassName(currentClasses) and getFilledClassName(currentClasses), but it's certainly less code than the way I probably would have written it if I had done so from scratch -- which would be to make a switch on the element's id and return the correct classes as a string accordingly.

\$\endgroup\$
1
\$\begingroup\$

As was already mentioned, the code mixes a lot of jQuery methods with plain vanilla DOM access JS methods. If you are going to use jQuery then why not use other jQuery methods? I was initially thinking .toggleClass() would work to simplify the logic on adding/removing class names, but because they are each somewhat unique that doesn't appear to work.

It is wise to store DOM lookups in a variable and then reference the variable instead of repeatedly querying the DOM. For instance, $('.canvas_btn') can be stored once the DOM is ready (using $(function() {...} (formerly .ready()):

$(function() { //DOM ready callback
  var canvasBtns = $('.canvas_btn');

Then use that variable instead of $('.canvas_btn'):

canvasBtns.click(function() { //click handler
  canvasBtns.each(function() { //iterate over canvas buttons

And as that last code snippet alludes to, use .each() (similar to Array.prototype.forEach() but for jQuery collections) to iterate over the buttons.

As far as updating the class name for each button, what you really want to do is remove -outline if the id matches the button that was clicked, or add it if it doesn't already have that. So in the rewritten code below, the id of the button that was clicked is stored in a variable, and then that is passed to the .each handler, which uses a partially applied function with Function.bind().

canvasBtns.click(function() {
  var clickedId = $(this).attr('id');
  canvasBtns.each(toggleOutlineClass.bind(null, clickedId));
});

Then that function toggleOutlineClass will accept first clickedId, followed by the other arguments passed by .each():

Integer index, Element element

Then that function can either remove the -outline string if it is the item clicked (based on whether clickedId matches $(btn).attr('id')) or add that string if the class name doesn't already have it.

function toggleOutlineClass(clickedId, index, btn) {
  var className = $(btn).attr('class');
  if (clickedId === $(btn).attr('id')) {
    $(btn).attr('class', className.replace('btn-outline-', 'btn-'));
  } else if (className.indexOf('btn-outline-') < 0) {
    $(btn).attr('class', className.replace('btn-', 'btn-outline-'));
  }
}

A lot of the tips above are based on information in this article. I suggest reading that for more tips.

$(function() { //DOM ready callback
  var canvasBtns = $('.canvas_btn');
  canvasBtns.click(function() {
    var clickedId = $(this).attr('id');
    canvasBtns.each(toggleOutlineClass.bind(null, clickedId));
  });

  function toggleOutlineClass(clickedId, index, btn) {
    var className = $(btn).attr('class');
    if (clickedId === $(btn).attr('id')) {
      $(btn).attr('class', className.replace('btn-outline-', 'btn-'));
    } else if (className.indexOf('btn-outline-') < 0) {
      $(btn).attr('class', className.replace('btn-', 'btn-outline-'));
    }
  }
});
<link href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" rel="stylesheet" />
<script src="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/js/bootstrap.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<button class="btn btn-outline-info 	  	canvas_btn" align="left" id="new_text_btn">Text</button>
<button class="btn btn-outline-secondary  canvas_btn" align="left" id="new_audio_btn">Audio</button>
<button class="btn btn-outline-success 		canvas_btn" align="left" id="new_image_btn">Image</button>
<button class="btn btn-outline-danger 		canvas_btn" align="left" id="new_video_btn">Video</button>
<button class="btn btn-outline-warning 		canvas_btn" align="left" id="new_input_btn">Input</button>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.