onClick on "a" tag not working properly if it has some child elements in react

Question

Below is the dynamic form which I created. It works fine. but Inside "a" tag if i add some child element to "a" tag, onClick event on "a" tag does not execute properly and pass proper name.

  import React, { Component } from "react";
import "./stylesheet/style.css";

export default class Main extends Component {
  state = {

    skills: [""]
  };



  dynamicInputHandler = (e, index) => {
    var targetName = e.target.name;
    console.log(targetName);

    var values = [...this.state[targetName]];
    values[index] = e.target.value;
    this.setState({
      [targetName]: values
    });
  };

  addHandler = (e, index) => {
    e.preventDefault();
    let targetName = e.target.name;
    let values = [...this.state[targetName]];
    values.push("");
    this.setState({
      [targetName]: values
    });
  };

  removeHandler = (e, index) => {
    e.preventDefault();
    let targetName = e.target.name;
    console.log(e.target.name);

    let values = [...this.state[targetName]];

    values.splice(index, 1);
    this.setState({
      [targetName]: values
    });
  };

  render() {
    return (
      <div>
        <form className="form">

          {this.state.skills.map((value, index) => {
            return (
              <div className="input-row row">
                <div className="dynamic-input">
                  <input
                    type="text"
                    placeholder="Enter skill"
                    name="skills"
                    onChange={e => {
                      this.dynamicInputHandler(e, index);
                    }}
                    value={this.state.skills[index]}
                  />
                </div>
                <div>
                  <span>
                    <a
                      name="skills"
                      className="close"
                      onClick={e => {
                        this.removeHandler(e, index);
                      }}
                    >
                      Remove
                    </a>
                  </span>
                </div>
              </div>
            );
          })}
          <button
            name="skills"
            onClick={e => {
              this.addHandler(e);
            }}
          >
            Add
          </button>
        </form>
        {this.state.skills[0]}
      </div>
    );
  }
}

i want to add Icon inside "a" tag , after adding icon tag, forms breaks and gives error "TypeError: Invalid attempt to spread non-iterable instance"

This works -

          <span>
                <a
                  name="skills"
                  className="close"
                  onClick={e => {
                    this.removeHandler(e, index);
                  }}
                >
                  Remove
                </a>
              </span>

This does not (after adding icon inside a tag)

       <span>
                <a
                  name="skills"
                  className="close"
                  onClick={e => {
                    this.removeHandler(e, index);
                  }}
                >
                  <i>Remove</i>
                </a>
              </span>

score 2 · Answer 1 · answered Jan 17 '19 at 21:30

I think this mixture of DOM manipulation (getting target of an event) is okay, but in React, where data is preferred over DOM values, you could work entirely on the component state and touch the DOM element values only once in the input where the skill value is displayed. Like this:

class Main extends Component {
  state = {
    skills: [],
    lastSkillAdded: 0
  };

  appendEmptySkill = () => {
    this.setState(prevState => ({
      ...prevState,
      skills: [...prevState.skills, { id: lastSkillAdded, value: "" }],
      lastSkillAdded: prevState.lastSkillAdded + 1
    }));
  };

  updateSkillById = (id, value) => {
    this.setState(prevState => ({
      ...prevState,
      skills: skills.map(skill =>
        skill.id !== id
          ? skill
          : {
              id,
              value
            }
      )
    }));
  };

  removeSkillById = id => {
    this.setState(prevState => ({
      ...prevState,
      skills: prevState.skills.filter(skill => skill.id !== id)
    }));
  };

  render() {
    return (
      <form>
        {this.state.skills.map((skill, index) => (
          <div key={skill.id}>
            <input
              type="text"
              value={skill.value}
              onChange={e => this.updateSkillById(skill.id, e.target.value)}
            />
            <button onClick={() => this.removeSkillById(skill.id)}>
              Remove
            </button>
          </div>
        ))}
        <button onClick={() => this.appendEmptySkill()}>Add</button>
      </form>
    );
  }
}

Let's deconstruct what's going on there.

First, the lastSkillAdded. This is a controversial thing, and I guess someone will correct me if I'm wrong, but

when we iterate over an array to render, say, list items, it's recommended that we provide key prop that represents the item key,
and the value of key prop is not recommended to be the index of the array element.

So we introduce an artificial counter that only goes up and never goes down, thus never repeating. That's a minor improvement though.

Next, we have the skills property of component state. It is where all the values are going to be stored. We agree that each skill is represented by an object of two properties: id and value. The id property is for React to mark array items property and optimize reconciliation; the value is actual text value of a skill.

In addition, we have three methods to work on the list of skills that are represented by three component methods:

appendEmptySkill to add an empty skill to the end of the state.skills array,
updateSkillById to update the value of a skill knowing id and new value of it,
and removeSkillById, to just remove the skill by its id from state.skills.

Let's walk through each of them.

The first one is where we just append a new empty skill:

appendEmptySkill = () => {
  this.setState(prevState => ({
    ...prevState,
    skills: [...prevState.skills, { id: lastSkillAdded, value: "" }],
    lastSkillAdded: prevState.lastSkillAdded + 1
  }));
};

Because appending a new skill never depends on any input values, this method takes zero arguments. It updates the skill property of component state:

[...prevState.skills, { id: lastSkillAdded, value: '' }]

which returns a new array with always the same empty skill. There, we also assign the value to id property of a skill to our unique counter.

The next one is a bit more interesting:

updateSkillById = (id, value) => {
  this.setState(prevState => ({
    ...prevState,
    skills: skills.map(skill =>
      skill.id !== id
        ? skill
        : {
            id,
            value
          }
    )
  }));
};

We agree that in order to change a skill, we need to know its id and the new value. So our method needs to receive these two values. We then map through the skills, find the one with the right id, and change its value. A pattern

(id, value) => array.map(item => item.id !== id ? item : ({ ...item, value }));

is, I believe, a pretty common one and is useful on many different occasions.

Note that when we update a skill, we don't bother incrementing our skill id counter. Because we're not adding one, it makes sense to just keep it at its original value.

And finally, removing a skill:

removeSkillById = id => {
  this.setState(prevState => ({
    ...prevState,
    skills: prevState.skills.filter(skill => skill.id !== id)
  }));
};

Here, we only need to know the id of a skill to remove it from the list of skills. We filter our state.skills array and remove the one skill that matches the id. This is also a pretty common pattern.

Finally, in render, four things are happening:

existing skills are iterated over, and for each skill, input and "Remove" button are rendered,
input listens to change event and refers to EventTarget#value attribute in updateSkillById function call, because that's the only way to feed data from DOM back into React, and
the "Remove" buttons refers to skill id and calls removeSkillById,
and outside the loop, the "Add" button listens to click events and calls appendEmptySkill whenever it happens without any arguments, because we don't need any.

So as you can see, this way you're in total control of the render and state updates and never depend on DOM structure. My general advice is, if you find yourself in a situation when DOM structure dictates the way you manage component or app state, just think about a way to separate DOM from state management. So hope this answer helps you solve the issue you're having. Cheers!

Thanks for such a brief explanation, I liked your approach but that would require a lot of refactoring in existing code. Thanks for your time and reply, I will remember this approach when building such forms from scratch — Akash Salunkhe, Jan 18 '19 at 10:14

Benjamin · Accepted Answer · 2019-01-19T05:21:16.387

2

Inside removeHandler let targetName = e.target.parentElement.name;. When you wrap Remove in another tag, the event target is now the i tag, for which name is undefined.

edited Jan 19 '19 at 05:21

answered Jan 17 '19 at 21:49

Benjamin

36
4

onClick on "a" tag not working properly if it has some child elements in react

2 Answers2