0

I have this component that loads some data with buttons. I have a function that gets button via ID and updates state/variable to that buttons value, then loads data based on that. However since all the button IDs are the same, it only works for the first button...

I'm not sure how to set unique IDs for each button since they're generated through a map() function, and even if i did know, i'm not sure how my function would then target each button without writing a function for each one...

Edit: I've set unique IDs for buttons now, still need to solve the other problem.

  return member.map((player, index) => (
    <>
      {" "}
      <React.Fragment key={index}>
        <br />{" "}
        <div>
          <br />
          {player.map((stats) => (
            <React.Fragment key={stats.player}>
              <div class="flex-child">
                <button id={'btn' + index} value={stats.player}>
                  {stats.player}
                </button>
                <p>{stats.role}</p>
                <p>{stats.win_rate}</p>
                <p>{stats.matches}</p>
                <p>{stats.total_battles} Total Battles</p>
                <p>{stats.score} / Points Earned</p>
              </div>
            </React.Fragment>
          ))}
        </div>
        <br />
      </React.Fragment>
    </>
  ));
};

export default SquadMembers;

here is index.js

import Head from "next/head";
import React, { useState } from "react";
import Teams from "../components/Teams";
import styles from "../../styles/Home.module.css";
import SquadMembers from "../components/SquadMembers";
import SquadData from "../components/SquadData";

export default function Home() {
  const [teams, setTeams] = useState([]);
  const [squads, setSquads] = useState([]);
  const [player, setPlayer] = useState("Player Name");
  const [squad, setSquad] = useState("default");
  const [loading, setLoading] = useState(false);

  function clicky() {
    if (!document.getElementById("btn")) {
    } else {
      setPlayer(document.getElementById("btn").value);
      loadPeople();
    }
  }

  if (typeof window === "object") {
    if (!document.getElementById("btn")) {
    } else {
      document.getElementById("btn").onclick = function () {
        clicky();
      };
    }
  }

  function handleChange(e) {
    setSquad(e.target.value);
  }

  const loadPeople = async () => {
    setLoading(true);
    const req = await fetch(`/api/player/${player}`);
    const json = await req.json();
    setTeams(json);
    setLoading(false);
  };

  const loadSquad = async () => {
    setLoading(true);
    const req = await fetch(`/api/squad/${squad}`);
    const json = await req.json();
    setSquads(json);
    setLoading(false);
    setTeams([]);
  };

  return (
    <div className={styles.main}>
      <main className={styles.main}>
        <h1>Silph Team Finder</h1>
        <br />
        <div>
          <select value={squad} onChange={handleChange}>
            <option value="default" selected disabled hidden>
              Choose here
            </option>
            <option value="a342">Lots of options, cut them to save space</option>
          </select>
          <button onClick={() => loadSquad()}>Load</button>

          <input
            value={player}
            id="player"
            onChange={(e) => setPlayer(e.target.value)}
          />
          <button onClick={() => loadPeople()} id="pbtn">
            Load
          </button>
          {loading && <div className={styles.load}>LOADING</div>}
        </div>
        <div className={styles.teams}>
          <SquadData squadz={squads} />
          <Teams teams={teams} />
          <div class="player-container">
            <SquadMembers squadz={squads} />
          </div>
        </div>
      </main>
    </div>
  );
}

Liiaam93
  • 195
  • 1
  • 2
  • 10
  • 1
    you can base id on eg index player.map((stats, idx) => ( // and then use idx variable in it) – farincz Jul 20 '21 at 13:42
  • You can get a list of all elements with the `btn` id using `document.querySelectorAll("#btn")` – lejlun Jul 20 '21 at 13:48
  • 2
    First of all you should not have two or more elements with the same ID within the same page; IDs must be unique. Secondarily, you can select all you buttons using a different selector that can be duplicated (e.g. a `class`\`className` attribute). Last but not least, if you are working with React you should not access a DOM node directly. – secan Jul 20 '21 at 13:49
  • Also, it looks like you are using `
    ` elements to add spacing rather than styling padding and margin via CSS; that is a bad practice.
    – secan Jul 20 '21 at 13:51
  • Perhaps you should first read this: [Does ID have to be unique in the whole page?](https://stackoverflow.com/q/9454645/11613622) – brc-dd Jul 20 '21 at 21:02
  • @farincz thanks! thats good to know, i just changed my code and now they have unique IDs :) – Liiaam93 Jul 21 '21 at 07:00
  • @secan I've updated the button IDs to be unique, originally i called the onclick function inside the button tag in the component, however it doesn't have access to the state variables and functions that are in index.js so i rewrote it to access the DOM instead from index.js - wasn't sure how else to go about it. Thanks for the help! – Liiaam93 Jul 21 '21 at 07:04
  • What is the button click supposed to do? Is it supposed to trigger the `clicky()` function (which will call `setPlayers()` and `loadPeople()`)? To be honest, the code you posted is not very self-explanatory and, as I mentioned before, it contains some bad practices both in terms of React code and in terms of the generated HTML code. [...] – secan Jul 21 '21 at 08:15
  • [...] Anyway, as a general rule, rather than attaching an event listener to the DOM button element, you should pass a callback function as a prop to the component that renders the button and execute the function on click: ` – secan Jul 21 '21 at 08:19
  • Maybe this example (which is not at all optimize and should not be used "as it is") can help you understand how the code should be organized and data should flow from a component to the other: https://jsfiddle.net/ukqthgbo/1/. As you can see, a callback function (`handleClick()`) is declared in the parent component and passed as prop to the child component, which will execute it (` – secan Jul 21 '21 at 08:54
  • @secan thanks again! I think i may have to stop and take a good look at react/next and even JS- I jumped into it without knowing much and i still feel like a beginner to JS. I'll take a look at that example now and hopefully i can start to learn more :) – Liiaam93 Jul 21 '21 at 11:12

1 Answers1

1

Would be much easier to have something like:

<button value={stats.player} onClick={() => handleClick(stats.player)}>
   {stats.player}
</button>

...

const handleClick = (player) => {
   setPlayer(player);
   loadPeople();
}

In this way you don't need id for button. Not only but you will avoid warning on same id for multiple elements.

Then I would like to talk about loadPeople(). If I understood correctly in this function you are using player that would be setted by setPlayer. This is not a good approach. setPlayer is async and you could take an old value of player. Much better pass the last player value directly to loadPeople function. Something like:

const handleClick = (player) => {
   setPlayer(player);
   loadPeople(player);
}

const loadPeople = async (newPlayer) => {
   setLoading(true);
   const req = await fetch(`/api/player/${newPlayer}`);
   const json = await req.json();
   setTeams(json);
   setLoading(false);
};
Giovanni Esposito
  • 10,696
  • 1
  • 14
  • 30
  • Thanks for the reply! I am very new to Next and still pretty new to JS if i'm being honest. I did try this approach at first however the button is located in a separate component and doesn't have access to the functions / state variables from the index page. Thanks! I will have a look at that function and I will edit the main post, to include the entire index page. – Liiaam93 Jul 21 '21 at 06:42