0

I am trying to make a button to check all checkboxes in a single line of a button onclick. Let's assume some checkboxes...

<input name="chk1" type="checkbox" class="post_dest_check" value="1" />Opt1
<input name="chk2" type="checkbox" class="post_dest_check" value="2" />Opt2
<input name="chk3" type="checkbox" class="post_dest_check" value="3" />Opt3

<button type="button" class="action" onClick="document.getElementsByClassName('post_dest_check').map((elm)=>{ elm.checked = true;});">Todos</button>

When clicked, will give me error

SyntaxError: Unexpected token ')'".

I also have tried with forEach.

Why? What's wrong? Is there a way to do this without heaving to write a script in the root to call from button?

Edit: I think I miss some explanation about why I wish to do like this, a script inside the onClick property. I have many forms that are loaded into the page as needed. By making a script in the page instead of in the form, I will have to create a "standard" to my forms, so the script can find the elements in the form every time is loaded. If I change one form, maybe I will have to change all of them and the script. Not a catastrophe, I just thought maybe a script inside the form can avoid this complexity - a built in automation. Of course, if possible and advisable.

Gustavo
  • 1,673
  • 4
  • 24
  • 39

2 Answers2

2

There are a few things wrong here:

  1. Do not use inline event attributes like onclick. Instead of using this 25+ year old technique that pre-dates standards and causes scalability and this binding issues, use the standard .addEventListener() that is separated from your HTML (which makes debugging easier by the way).

  2. Do not use getElementsByClassName() as this returns a live node list that can cause performance issues, especially in looping situations.

  3. .map() returns a new array, which in your use case is not needed and therefore a waste of resources. Instead, .forEach() is the better approach.

  4. Don't bother with self-terminating XHTML syntax. It buys you nothing and can add confusion as to where it can/can't be used.

In the code below, "event delegation" is used so that you can have one event handler handle any set of checkboxes that are within their own form.

// Set up an event handler at a common ancestor of all the elements you wish to handle
document.querySelector("section").addEventListener("click", function(event){
  // Check to see if the event was triggered by an element you care to handle
  if(event.target.classList.contains("action")){
    // Get your reference to all the checkboxes in that form
    // (.closest searches for the nearest matching ancestor) and
    // loop over the checkboxes with the .forEach() method that works on node lists
    event.target.closest("form").querySelectorAll("input.post_dest_check").forEach(function(box){
      box.checked = true;
    });
  }
});
<section>
<fieldset>
  <legend>Form 1</legend>
  <form>
    <input name="chk1" type="checkbox" class="post_dest_check" value="1">Opt1
    <input name="chk2" type="checkbox" class="post_dest_check" value="2">Opt2
    <input name="chk3" type="checkbox" class="post_dest_check" value="3">Opt3
    <button type="button" class="action">Todos</button>
 </form>
</fieldset>

<fieldset>
  <legend>Form 2</legend>
  <form>
    <input name="chk1" type="checkbox" class="post_dest_check" value="1">Opt1
    <input name="chk2" type="checkbox" class="post_dest_check" value="2">Opt2
    <input name="chk3" type="checkbox" class="post_dest_check" value="3">Opt3
    <input name="chk3" type="checkbox" class="post_dest_check" value="3">Opt4    
    <button type="button" class="action">Todos</button>
 </form>
</fieldset>

<fieldset>
  <legend>Form 3</legend>
  <form>
    <input name="chk1" type="checkbox" class="post_dest_check" value="1">Opt1
    <input name="chk2" type="checkbox" class="post_dest_check" value="2">Opt2
    <button type="button" class="action">Todos</button>
 </form>
</fieldset>
</section>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • This is precisely what I wish to avoid. The point is to load a form with this "build in" automation, since HTML5 don't have (as far I know) "check all" feature as clear form button. My goal is a script inside onClick property - is this possible? – Gustavo Apr 08 '22 at 20:23
  • @Gustavo I really don't know what you are saying. Why is this modern, standards-based approach what you want to avoid? What doesn't this solution do that you need it to? All you need to do is include the JavaScript within a ` – Scott Marcus Apr 08 '22 at 20:32
  • I have a lot of forms that will be load eventually, as needed. Is not a whole page loaded, just the form. So after loading I will have to add the event to the button and build the array, always to every form. I will have to create a "standard", a smart script that find and activate the automation. I'm not saying your solution is wrong, I just thought about avoiding this two documents solution for a single document, a form with build in automation that I can change later without to worry with the counterpart script. – Gustavo Apr 09 '22 at 13:08
  • I see. There's a simple solution for that called "event delegation" which allows for an event to bubble up to an ancestor element and be handled there. This way, you can write one handler for many elements without even knowing what they are. I will update the answer to demonstrate this. – Scott Marcus Apr 09 '22 at 16:05
  • Yes! Now you got it. I did put in my page ini but getting all body instead section. Now is just to load the for with the right classes and voilá, I have my check all button. I still thought would be better some script in the form. But I have to admit a script as @JD Byrnes suggested is kinda outside good practices, I was thinking about some hack, instead of a full code. – Gustavo Apr 09 '22 at 21:44
1

How to check all checkboxes using javascript in onClick button?

for(el of document.getElementsByTagName('input')){if(el.type==="checkbox")el.checked=true;}

However, if you only want checkboxes with the class 'post_dest_check' you could be a little more specific.

for(el of document.getElementsByClassName('post_dest_check')){if(el.constructor.name==="HTMLInputElement"&&el.type==="checkbox")el.checked=true;}

I've only left out optional spaces as you've indicated you were going to use this in an inline onClick event handler.

JD Byrnes
  • 783
  • 6
  • 17
  • Both `.,getElementsByTagName()` and `.getElementsByClassName()` return live node lists which adversely affect performance and should be avoided. – Scott Marcus Apr 09 '22 at 20:44
  • Thanks for the suggestion. I have changed a bit of my mind and decided to use the Scott's code. To put all this code inside a property is really... ugly, I must admit. I was thinking about some small code like a hack, that's why I was trying with map and forEach. – Gustavo Apr 09 '22 at 21:48
  • @ScottMarcus It's irrelevant to me about the minor performance impact. OP had indicated he *needed* to do it inline, and this was the best solution for that. – JD Byrnes Apr 10 '22 at 20:58
  • It's far from minor. Those are legacy methods and should be avoided. And, those two methods are hardly the only ones that can be used inline (`.querySelectorAll()`, `.querySelector()`). – Scott Marcus Apr 10 '22 at 21:31
  • Benchmarking shows the opposite of what you're stating, and it seems like the source for your references is... yourself? https://measurethat.net/Benchmarks/ShowResult/284968 On my computer I can do 9,264,681 calls to getElementsByClassName() per second, with only 8,001,476 per second to querySelector(). If you're gonna continue calling people out, feel free to add some reliable information. If you don't call -17.0401439 nanoseconds minor, then what is? – JD Byrnes Apr 11 '22 at 00:03
  • The "should be avoided" is a load of rubbish - They're not deprecated, or even discouraged. MDN (https://developer.mozilla.org/en-US/docs/Web/API/Document/getElementsByClassName) does show a warning about using Live HTMLCollections incorrectly when iterating, but this is a non-issue for this use case (we're synchronously looping the collection, which can not change while in this loop). – JD Byrnes Apr 11 '22 at 00:06
  • I did not say that they were deprecated, I said they were legacy. And there is a significant impact when used in loops. This is not rubbish it's real. And because most new (and a significant number of experienced) developers don't understand that impact, they use these methods all the time without any understanding of the difference between live and static node lists, which leads to issues when the code invariably scales. These methods fall into the anti-pattern group. But hey, go ahead and advocate for unstable code. – Scott Marcus Apr 11 '22 at 16:02
  • I have seen this happen virtually every time in the nearly 30 years of my experience. And your "benchmark" is hardly real world. I have benchmarked this in the past and with real-world code and the results are clear and significant. – Scott Marcus Apr 11 '22 at 16:02
  • Here's a [real world test case](https://measurethat.net/Benchmarks/Show/18216/0/getelementsbyclassname-vs-queryselectorall) which has `.getEelementsByClassName()` doing 34,670 ops/sec while `.querySelectorAll()` does 1,413,175 ops/sec. As you can see, `.querySelectorAll()` is orders of magnitude faster and we haven't even gotten into dynamically added/removed HTML elements, which is where a live node list comes in handy, but frankly, re-querying with `.querySelectorAll()` would still be faster. – Scott Marcus Apr 11 '22 at 16:15
  • By the way, you may not realize this, but you only incur overhead with live node lists when you access them, not when you set them up, so simply calling `.getElementsByClassName()` doesn't actually go and scan the DOM for matching elements. It's when you access the live node list returned from that call that the DOM is scanned (thus giving your the "Live" results). Your benchmark didn't actually use the live node list, which is why your results for it were comparable to `.querySelectorAll()` (which does actually scan the DOM when you use it). – Scott Marcus Apr 12 '22 at 13:11
  • Additionally, you compared `.querySelector()` against `.getElementsByClassName()`, when you should have been comparing `.querySelectorAll()` as I did in my benchmark. `.querySelector()` doesn't return a node list at all, it returns the first matching DOM element. – Scott Marcus Apr 12 '22 at 13:14