Same Javascript function working for two different divs

Question

I´ve got this javascript function working fine for mobile and i need to use it for desktop but the ids of the elements are not the same, i was wondering if someone could give me a hint on how to approach this.

function EventMailing() {
  var email = document.querySelector("#newsletter-popup fieldset #email").value;
  if (email == undefined) {
    email = "";
    if (email == undefined || email == "") return;
  }
  var rgxHome = new RegExp(".*?://(.*)someurl.com($|/|/?(.*))$");
  if (rgxHome.test(location.href)) {
    SendEmail(email);
    document.querySelector("#newsletter-popup fieldset #email").value = "";
    setTimeout(function () {
      $("#newsletter-popup").css("display", "none");
    }, 2000);

}

My Html code for mobile:

<div id="newsletter-popup" >
  <a class="close icon-button-close" href="#"></a>
  <h2>Suscribe to Newsletter</h2>
  <p>some text</p>
  <fieldset>
    <label for="email"> e-mail</label>
    <input id="email" name="email" type="text">
    <button onclick="EventMailing();null">OK</button>
  </fieldset>
</div>

Html code for desktop:

<div id="newsletterPopup" >
  <a class="close icon-button-close" href="#"></a>
  <h2>Suscribe to Newsletter</h2>
  <p>some text</p>
  <fieldset>
    <label for="email"> e-mail</label>
    <input id="emailp" name="email" type="text">
    <button onclick="EventMailing();null">OK</button>
  </fieldset>
</div>

I don't have access to the Html code so im trying to figure out a javascript solution.

You are selecting element `email` by its id and then you check if you found it or not. So in that check just write a second one for your pc version. `if (email == undefined || email == "") return;` over here instead of `return` just get the pc version and if its not found either just return — Jay Nyxed, Dec 29 '20 at 21:41
Are you allowed to edit the `HTML` at all? Or you’re just purely looking for a `JavaScript` solution? — goto, Dec 29 '20 at 22:00
@iota, IDs must be unique. An ID that isn't doesn't ID anything. This is in the HTML spec. — isherwood, Dec 29 '20 at 22:03
@isherwood The questions says that different HTML is used for mobile and desktop, with different ids given to the main container. — Unmitigated, Dec 29 '20 at 22:04
Doesn't matter. Same document (I assume--OP hasn't really made it clear what's happening here). — isherwood, Dec 29 '20 at 22:04
A better question is **why are you duplicating the markup?** It appears virtually identical. You should probably be reusing it. Otherwise, an event listener for a _class_ of container elements would resolve this in a hurry. — isherwood, Dec 29 '20 at 22:05
I don't have access to the Html code so im trying to figure out a javascript solution. — ruizwau, Dec 29 '20 at 22:13

Scott Marcus · Accepted Answer · 2020-12-29T22:00:10.107

Since the element you want to work with is the prior sibling element of the button that was clicked, just use the .previousElementSibling property of the button that triggered the event to get the proper reference without using an id at all. However, you should not use inline event handlers, like onclick for several reasons, one of which is that the this binding isn't always correctly bound. Instead, use the modern .addEventListener().

As you've found, working with ids creates brittle code that doesn't scale well. Although they are easy to use, at first. As you develop more complex code, you'll find they cause more problems than they solve. There are many other ways to locate the correct elements that use relative navigation or locate based on CSS queries and that is what you should be using.

document.querySelector("button").addEventListener("click", EventMailing);

function EventMailing() {
  var email = this.previousElementSibling;
  console.log(email.value);
}

<div id="newsletterPopup" >
  <a class="close icon-button-close" href="#"></a>
  <h2>Suscribe to Newsletter</h2>
  <p>some text</p>
  <fieldset>
    <label for="email"> e-mail</label>
    <input id="emailp" name="email" type="text">
    <button>OK</button>
  </fieldset>
</div>

Another solution would be to use Event Delegation as shown and documented below:

// Insted of setting up specific event handlers,
// you can leverage event delegation and let the
// event bubble up to some ancestor element where
// it is handled. Here, we're setting up a click
// handler on the document.
document.addEventListener("click", EventMailing);

function EventMailing(event) {
  // All event handlers are automatically passed
  // a reference to the event object that triggered
  // them. You can inspect the .target of the event
  // which references the actual object that triggered
  // the event
  if(event.target.nodeName === "BUTTON"){
    var email = event.target.previousElementSibling;
    console.log(email.value);
  } 
}

<div id="newsletter-popup" >
  <a class="close icon-button-close" href="#"></a>
  <h2>Suscribe to Newsletter</h2>
  <p>some text</p>
  <fieldset>
    <label for="email"> e-mail</label>
    <input id="email" name="email" type="text">
    <button>OK</button>
  </fieldset>
</div>

<div id="newsletterPopup" >
  <a class="close icon-button-close" href="#"></a>
  <h2>Suscribe to Newsletter</h2>
  <p>some text</p>
  <fieldset>
    <label for="email"> e-mail</label>
    <input id="emailp" name="email" type="text">
    <button>OK</button>
  </fieldset>
</div>

The problem is i can't edit the Html code because i don't have access to it. — ruizwau, Dec 29 '20 at 22:09
@ruizwau Who does have access because it's outdated and shouldn't be written that way. Even the `label for` is incorrect because `for` should reference an `id` of an element, not its `name` (which was the old way it used to work). — Scott Marcus, Dec 29 '20 at 22:11

Same Javascript function working for two different divs

1 Answers1