1

I have the following bit about shop info:

<h3 class="-center"><shop>Shop name</shop></h3>
<button type="button" id="banff" onclick="showShop(this.id)"><shop-title><b>Bakery Shop</b></shop-title></button>
<p class="move-right"><shop-info>Shop Address · Shop number</shop-info></p>
  <div id="shop"  class="hidden">
    <p><shop-info>Opening soon</shop-info></p>
  </div>

What I'm trying to do is that once the button is clicked, it will get the first div after the button and toggle the hidden class without referencing the id, there will be multiple shops so I am trying to create one function which will work for all of them rather than create an individual function for each.

I thought I could do something like:

function showShop(id) {
const elem = document.getElementById(id);
 alert(id);
const div = elem.closest('div');
div.classList.toggle('hidden'); 

}

But it's referencing the first div on the page rather than the first one after the button, where have I gone wrong here? Or do I need to go about this a different way?

Thanks in advance for any advice

l15
  • 65
  • 1
  • 7
  • 2
    You cannot have more than one element in the page with any given `id`. `id` must be unique. Please also note that `` is not valid HMTL either. `closest` only searches *ancestors*. Also `showShop(this.id)` doesn't make sense because the button does not have an `id`. – connexo Jun 04 '22 at 16:50
  • apologies, the button does have an id, left that out by mistake – l15 Jun 04 '22 at 16:57
  • and in terms of the custom tags, from what I've seen there is no issue using your own ones if that's the route you choose to go down – l15 Jun 04 '22 at 16:58
  • 1
    There is no pro argument when it comes to invalid HTML, it's always unacceptable. – connexo Jun 04 '22 at 17:08
  • 1
    I've seen other people saying it's fine, like [here](https://stackoverflow.com/questions/37622348/is-it-bad-practice-to-define-a-custom-html-tag#:~:text=Creating%20custom%20HTML%20tags%20are,when%20working%20with%20web%20components.) . That wasn't what my question is about and don't really have any interest in arguing with you about it, if you don't like it fair enough but I'll carry on using them like many other people do – l15 Jun 04 '22 at 17:18

2 Answers2

1

What about connecting the button id with the div id?

something like:

  <h3 class="-center"><shop>Shop name</shop></h3>
  <button type="button" id="button-1" onclick="showShop(this.id)">
    <shop-title><b>Bakery Shop</b></shop-title>
  </button>
  <p class="move-right"><shop-info>Shop Address · Shop number</shop-info></p>
  <div id="shop-button-1" class="hidden">
   <p><shop-info>Opening soon</shop-info></p>
  </div>

and in the javascript code would be possible to simplify to:

function showShop(id) {
  const elem = document.getElementById(id);
  const div = document.getElementById('shop-' + id);

  div.classList.toggle('hidden'); 
}
samuel silva
  • 432
  • 3
  • 8
1

You should try and avoid using ids as they are difficult to keep track of. Instead you should navigate relative to the clicked button. Below is a script that can easily take care of multiple shop sections without using any ids:

document.querySelectorAll("button").forEach(btn=>{
  for(var div=btn;div=div.nextElementSibling;)
    if(div.tagName==="DIV") break;
  if(div) btn.onclick=()=>div.classList.toggle("hidden")
})
.hidden {display:none}
<h3 class="-center"><shop>Shop name</shop></h3>
<button><shop-title><b>Bakery Shop</b></shop-title></button>
<p class="move-right"><shop-info>Shop Address · Shop number</shop-info></p>
  <div class="hidden">Info on bakery shop:
<p><shop-info>Opening soon</shop-info></p>
  </div>
<button><shop-title><b>Iron Monger</b></shop-title></button>
<p class="move-right"><shop-info>Shop Address · Shop number</shop-info></p>
  <div class="hidden">Info on iron monger's:
<p><shop-info>already open!</shop-info></p>
  </div>
<button><shop-title><b>Fish Monger</b></shop-title></button>
<p class="move-right"><shop-info>Shop Address · Shop number</shop-info></p>
  <div class="hidden">Info on fish monger's:
<p><shop-info>will never open!</shop-info></p>
  </div>

In my latest update I tweaked the snippet again so that now I do the "relative navigation to the associated <div>" only once: at the time when I define and add the onclick eventlistener.

Carsten Massmann
  • 26,510
  • 2
  • 22
  • 43