Search code examples
javascriptbuttoninnerhtmldivider

Need help solving a javascript issue with a button being clicked that is not being clicked

Ok, so I have a web interface I'm designing that I'm having a problem with. It's an interface to configure remotes to control functions of an ESP32. The code on the ESP32 is definitely not the problem here, it's something in the javascript that I am fairly new to for sure. Here's the process... The web interface has kind of a tab structure (menu at top, dividers for each "tab" that are hidden or shown depending on which menu item you click) So, there's a remote configuration tab (). When the page is loaded, the script sends a websocket command to the controller requesting the remote configuration. The controller then sends back data which my javascript takes and puts into the innerhtml of the divider containing an html table list of the remotes paired with the device and edit/delete buttons for each one as well as a back button.

Delete buttons are currently not implemented as I want to get everything else working before deleting anything.

The edit button passes a command to the controller with then loads a new table containing some editable data and a list of the buttons the remote has into the innerhtml of the divider.

This list of buttons also has edit and delete functions which are pretty similar to the above as well as a back button.

The back button on the "edit button" function is what is giving me a hard time. Basically, the back button runs the same function as the edit remote function reloading the remote configuration and list of buttons into the innerhtml.

Here's the real problem...

I click the edit on a remote, it loads the remote config and list of buttons. I click the edit on a button, it loads the button configuration. I click back, and....

script sends command to edit remote then... script sends command to edit button

It should not be sending the edit button command.

I've included a link below to reproduce this issue without you having the controller itself. I've also added alert messages to make it clear what is happening.

Any help would be appreciated.

Link to JSFiddle example: https://jsfiddle.net/kb1sph/37epovwg/

My code is too big for this post, but to add a jsfiddle link I need a code snippet...so here's a dummy code snippet.

Solution

  • If you delegate (you need to target the button itself) you do not need to loop over the buttons.

    Added benefit, which solves one of your issues: If you add the buttons later, the container will still handle the event

    The other issue was that you likely added eventHandlers more than once to the same buttons

    const remotegrid = document.getElementById('remotegrid');
remotegrid.addEventListener("click", (e) => {
  const tgt = e.target.closest("button"); // you can click anywhere on the buttom
  if      (tgt.matches(".remedit")) editrem(tgt);
  else if (tgt.matches(".btnedit")) editbtn(tgt);
  else if (tgt.matches(".remdel"))  delrem(tgt);
  else if (tgt.matches(".btndel"))  delbtn(tgt);
});

    To shorten your code, you can load fontawesome onto the device and use code like <i class="fal fa-pencil"></i> instead of the SVG

    or even smaller:

     <button class='btnedit' title='Edit Button'>✎</button>
 <button class='btndel' title='Delete Button'>🗑</button>

    How about changing this

    function ChangeTab(evt, tabName) {
  var i, tabcontent, tablinks;
  tabcontent = document.getElementsByClassName("tabcontent");
  for (i = 0; i < tabcontent.length; i++) {
    tabcontent[i].style.display = "none";
  }
  tablinks = document.getElementsByClassName("tablinks");
  for (i = 0; i < tablinks.length; i++) {
    tablinks[i].className = tablinks[i].className.replace(" active", "");
  }
  document.getElementById(tabName).style.display = "grid";
  evt.currentTarget.className += " active";
}

    to this much more readable code. Also it uses native classlist manipulation instead of an error prone replace with commas and spaces

    const ChangeTab = (evt, tabName) => {
  const currenttab = evt.currentTarget;
  document.querySelectorAll(".tabcontent")
    .forEach(content => content.style.display = "none");
  document.querySelectorAll(".tablinks")
    .forEach(tablink => tablink.classList.toggle("active",tablink === currenttab));
  document.getElementById(tabName).style.display = "grid";
};

    Here is another improved event handler

    remotegrid.addEventListener("change", (e) => {
  const tgt = e.target;
  if (!tgt.matches("[name=remoteenabled]")) return; 
  SendCmd(`<RemoteEnabled MAC="${tgt.id}" Enabled="${tgt.value}"/>`);
});

    I have added many changes to https://jsfiddle.net/mplungjan/vn6gh2jx/

    I may have broken some functionality, but it does not error

    To shorten it even more, look into template elements