Skip to main content
added 5 characters in body
Source Link
Dan
  • 3.8k
  • 24
  • 39
Click!
<form>
  <input name="mode" value="...." />
  <input name="type" value="...." />
  <button data-my-widget data-mode="mode" data-value="type">Click!</button>
</form>
Click!
<form>
  <input name="mode" value="...." />
  <input name="type" value="...." />
  <button data-my-widget data-mode="mode" data-value="type">Click!</button>
</form>
added 1267 characters in body
Source Link
Dan
  • 3.8k
  • 24
  • 39

My core recommendation would not be to rely on IDs as they are unique per page. Instead, consider relying on data attributes, and never assuming that there are a particular number of them. Your javascript should be as disconnected from your HTML layout as possible.

Let's modify what you have a bit. Let's start with adding a data attribute that indicates to JavaScript that you have a special button:

const myThing = event => {
  const outputs = document.getElementsByTagName('output');
  if (outputs.length === 0) {
    return;
  }

  const output = outputs[0];
  const target = event.target;
  const { form } = event.target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId } = eventtarget.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  if (mode === undefined || value === undefined || value in options === false) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }
  output.innerHTML = f();
}

This keeps your javascript as unaware of your HTML structure as possible, although I'd still recommend replacing the outputs section as well -, potentially by again providing an id tousing data attributes, rather than relying on the buttonindex of the element:

<output id="output-1"></output> 
<button ... data-output="output-1">Click!</button>

const myThing = event => {
  ...
  const { mode: modeId, value: valueId, output: outputId } = target.dataset;
  ...
  const output = document.getElementById(outputId);
  output.innerHTML = ...;
}

Putting this all together:

<output id="output-1"></output>
...
<output id="output-n"></output>
<form>
  <input name="mode" value="...." />
  <input name="type" value="...." />
  <button data-my-widget data-mode="mode" data-value="type" data-output="output-1">Click!</button> 
</form>

const myThinghandleClick = event => {
  ..const { form } = event.target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId, output: outputId } = eventtarget.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  const output = document.getElementById(outputId);
  if (mode === undefined || value === undefined || output === undefined) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }

  output.innerHTML = .f();
}

for (const button of document.querySelectorAll('[data-my-widget]')) {
  button.addEventListener('click', handleClick);
}

My core recommendation would not be to rely on IDs as they are unique per page. Instead, consider relying on data attributes, and never assuming that there are a particular number of them. Let's modify what you have a bit. Let's start with adding a data attribute that indicates to JavaScript that you have a special button:

const myThing = event => {
  const outputs = document.getElementsByTagName('output');
  if (outputs.length === 0) {
    return;
  }

  const { form } = event.target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId } = event.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  if (mode === undefined || value === undefined || value in options === false) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }
  output.innerHTML = f();
}

This keeps your javascript as unaware of your HTML structure as possible, although I'd still recommend replacing the outputs section as well - potentially by again providing an id to the button:

<output id="output-1"></output>
....

<button data-my-widget data-mode="mode" data-value="type" data-output="output-1">Click!</button> 

const myThing = event => {
  ...
  const { mode: modeId, value: valueId, output: outputId } = event.dataset;
  const output = document.getElementById(outputId);
  output.innerHTML = ...;
}

My core recommendation would not be to rely on IDs as they are unique per page. Instead, consider relying on data attributes, and never assuming that there are a particular number of them. Your javascript should be as disconnected from your HTML layout as possible.

Let's modify what you have a bit. Let's start with adding a data attribute that indicates to JavaScript that you have a special button:

const myThing = event => {
  const outputs = document.getElementsByTagName('output');
  if (outputs.length === 0) {
    return;
  }

  const output = outputs[0];
  const target = event.target;
  const { form } = target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId } = target.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  if (mode === undefined || value === undefined || value in options === false) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }
  output.innerHTML = f();
}

This keeps your javascript as unaware of your HTML structure as possible, although I'd still recommend replacing the outputs section as well, potentially by using data attributes, rather than relying on the index of the element:

<output id="output-1"></output> 
<button ... data-output="output-1">Click!</button>

const myThing = event => {
  ...
  const { mode: modeId, value: valueId, output: outputId } = target.dataset;
  ...
  const output = document.getElementById(outputId);
  output.innerHTML = ...;
}

Putting this all together:

<output id="output-1"></output>
...
<output id="output-n"></output>
<form>
  <input name="mode" value="...." />
  <input name="type" value="...." />
  <button data-my-widget data-mode="mode" data-value="type" data-output="output-1">Click!</button>
</form>

const handleClick = event => {
  const { form } = event.target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId, output: outputId } = target.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  const output = document.getElementById(outputId);
  if (mode === undefined || value === undefined || output === undefined) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }

  output.innerHTML = f();
}

for (const button of document.querySelectorAll('[data-my-widget]')) {
  button.addEventListener('click', handleClick);
}
Source Link
Dan
  • 3.8k
  • 24
  • 39

I assume your HTML looks a bit like this:

<output></output>
<output></output>
<form>
  <input name="mode" value="..." />
  <input name="type" value="..." />
  <button>Click!</button>
</form>

My core recommendation would not be to rely on IDs as they are unique per page. Instead, consider relying on data attributes, and never assuming that there are a particular number of them. Let's modify what you have a bit. Let's start with adding a data attribute that indicates to JavaScript that you have a special button:

<button data-my-widget>Click!</button>

This would enable you to assign an event listener to all buttons with the given attribute and would enable your buttons to remain progressively enhanced:

const myThing = event => {
  ...
}

for (const button of document.querySelectorAll('[data-my-widget]')) {
  button.addEventListener('click', newThing);
}

You are also hard-coding which form each button applies to - and the respective controls - within the event handler. You can make this a bit better by using the form attribute on a button, and by providing the target element ids as data attributes as well.:

Click!

Then you could access this form within the handler:

const myThing = event => {
  const outputs = document.getElementsByTagName('output');
  if (outputs.length === 0) {
    return;
  }

  const { form } = event.target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId } = event.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  if (mode === undefined || value === undefined || value in options === false) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }
  output.innerHTML = f();
}

This keeps your javascript as unaware of your HTML structure as possible, although I'd still recommend replacing the outputs section as well - potentially by again providing an id to the button:

<output id="output-1"></output>
....

<button data-my-widget data-mode="mode" data-value="type" data-output="output-1">Click!</button> 

const myThing = event => {
  ...
  const { mode: modeId, value: valueId, output: outputId } = event.dataset;
  const output = document.getElementById(outputId);
  output.innerHTML = ...;
}