I'm trying to make a CRUD person function where each person has an array of skills. I want a function where you're able to add/edit/remove skills on a given person. Each array consist of a skill element as a string and a star element as an integer. I've made some dynamic inputfields with an add and a remove function for more/less inputfields in a bootstrap modal.
The data is fetched from Firebase with a useEffect and set as setData in EditPerson.jsx. No problem here.
The issue consist of 3 components atm: EditPerson -> ModalEditSkills -> EditSkills. (Please let me know if this is a bad structure).
I'm now able to set the useState of newData in SkillEdit.jsx with the correct data. This makes sure that on EditPerson I'll be able to view the correct data input from given in the EditSkills. Also if I console.log the data in EditSkills I can see that it works like a charm. But when I close the bootstrap modal and open it again the useState in index 0 have been reset to init useState (0).
I can't add images in the text here yet, so here's some links for the images if needed. The image explains that the console.log tells me that the useState is set correct, but it stills reset the state of index 0 everytime I re-open the modal.
Hope that makes sense otherwise let me know.
EditPerson.jsx
const EditPerson = () => {
const [data, setData] = useState({});
const [skills, setSkills] = useState([]);
const { id } = useParams();
useEffect(() => {
if (id) {
const fetchData = async () => {
const docRef = doc(db, "person", id);
try {
const docSnap = await getDoc(docRef);
setData(docSnap.data());
} catch (error) {
console.log(error);
}
};
fetchData().catch(console.error);
} else {
setData("");
}
}, [id]);
useEffect(() => {
if (data) {
setSkills(data.skills);
}
}, [data]);
const handleSkills = (skill) => {
setSkills(skill);
};
return (
<div>
<ModalEditSkills
handleSkills={handleSkills}
data={skills}
/>
</div>
);
}
ModalEditSkills.jsx
const ModalEditSkills = ({ data, handleSkills }) => {
const [show, setShow] = useState(false);
const [newData, setNewData] = useState({});
useEffect(() => {
if (data) {
setNewData(data);
}
}, [data]);
const handleClose = () => setShow(false);
const handleShow = () => setShow(true);
const handleSubmitSkills = (e) => {
e.preventDefault();
handleSkills(newData);
setShow(false);
};
return (
<>
<div className="content_header">
<div className="content_header_top">
<div className="header_left">Skills</div>
<div className="header_right">
<Button className="round-btn" onClick={handleShow}>
<i className="fa-solid fa-pencil t-14"></i>
</Button>
</div>
</div>
</div>
<Modal show={show} onHide={handleClose} size="">
<Modal.Header closeButton>
<Modal.Title>Edit Person</Modal.Title>
</Modal.Header>
<Modal.Body>
<SkillEdit data={data} setNewData={setNewData} />
</Modal.Body>
<Modal.Footer>
<Form>
<Button className="btn-skill-complete" onClick={handleSubmitSkills}>
Save
</Button>
</Form>
</Modal.Footer>
</Modal>
</>
);
};
SkillEdit.jsx
const SkillEdit = ({ data, setNewData }) => {
const [inputField, setInputField] = useState([{ skill: "", stars: 0 }]);
const handleAddFields = () => {
setInputField([...inputField, { skill: "", stars: 0 }]);
};
const handleRemoveFields = (index) => {
const values = [...inputField];
values.splice(index, 1);
setInputField(values);
setNewData(values);
};
const handleChangeInput = (index, name, value) => {
const values = [...inputField];
values[index][name] = value;
setInputField(values);
setNewData(values);
};
useEffect(() => {
if (data) {
const setSkills = () => {
setInputField(data);
};
setSkills();
}
}, [data]);
return (
<Form>
<div>
{inputField?.map((inputField, index) => (
<div key={index}>
<Row>
<Col xs={5} md={5}>
<Form.Group as={Col}>
<Form.Control
className="mb-3"
type="text"
id="skill"
name="skill"
value={inputField?.skill}
onChange={(event) =>
handleChangeInput(index, "skill", event.target.value)
}
/>
</Form.Group>
</Col>
<Col xs={4} md={4}>
<div>
<Form.Group>
<ReactStars
type="number"
name="stars"
count={5}
size={24}
id="stars"
onChange={(newValue) =>
handleChangeInput(index, "stars", newValue)
}
emptyIcon={<i className="fa-solid fa-star"></i>}
filledIcon={<i className="fa-solid fa-star"></i>}
value={inputField.stars}
/>
</Form.Group>
</div>
</Col>
<Col xs={3} md={3}>
<div>
<button
type="button"
onClick={() => handleAddFields()}
>
<i className="fa-solid fa-plus"></i>
</button>
<button
type="button"
onClick={() => handleRemoveFields(index)}
>
<i className="fa-solid fa-minus"></i>
</button>
</div>
</Col>
</Row>
</div>
))}
</div>
</Form>
);
};
This took some time for me to work out. I had trouble reproducing and still do, but I noticed a lot of odd behaviour around the stars. In the end, I've figured out it's probably this bug in the react-stars package.
Unfortunately, the value prop does not actually control the value after the initial render. So it's like an uncontrolled component. The library therefore, is poor. It hasn't been committed to for 4 years. Usually, if a component is uncontrolled, the developer calls the prop initialValue or defaultValue instead of value, which usually implies the component is controlled. Here, the author has made a mistake. Regardless, in your case, you need controlled mode.
It's possible there's another bug interacting. But I'd start by replacing react-stars as not being able to have controlled mode is extremely poor and it makes it very hard to see the wood through the trees. There is a "solution" in the GitHub thread but it's a massive hack -- it's using the special key property to remount it every time the value changes.
I went looking for an alternative and much to my surprise a lot of the other libraries are also uncontrolled -- which really sucks. What you could do instead of the hack in the GitHub issue, is make it so the dialog is unmounted when open is false. This would mean each time the dialog opens it resets the value back to that which is held in the parent state. See my bottom code for that solution.
There's good options though here and here but they are part of larger design systems, and it's probably overkill to bring in a whole other design system when you have committed to bootstrap. Depending on how early you are in your project though, I'd seriously consider switching to something like MUI. Personal opinion territory, but Bootstrap is pretty outdated and the React wrapper and associated community, plus diversity of components, is much smaller. It shows that react-bootstrap is a wrapper on top of old school global CSS files as opposed to material-ui which was built from the ground up in React and has a React based CSS-in-JS solution. When people first start learning React, they often slip into bootstrap because it's what they know from non-React dev -- but using a framework like React moves the needle and trade-offs.
It's not your problem here, but I feel the need to say it :D. At the same time I'd say don't always take a random internet strangers recommendation and refactor for nothing -- you should research it.
A few other important notes:
id is not set, you set data to an empty string. If that case ever happened for real, your code would error since accessing data.skills would result in undefined. I would argue that you shouldn't even need to handle this case, since your router setup should be such that there is no mapping between this component and a route without an id. Make sure on your router config the id param is non-optional.data.skills into a new state item called skills. It's not a bug per se but it is a code smell as it's not actually necessary. Why not edit the data in the data state directly? Copying state that is fundamentally the same is usually an anti-pattern as you can fall into traps where you are trying to keep 2 bits of state in sync that are supposed to be the same. I think it's OK where you do it further down as I believe you are copying it because you want that state not to be committed further up until the user clicks save -- which means it's actually fundamentally different state at given times. But that doesn't apply to the top level data vs skills one. Actually, I think the inputField state is also not needed since your "staged" state is held by the parent ModalEditSkills in newData. You could instead get rid of this inputField state and have handleChangeInput, handleAddFields and handleRemoveFields call up into ModalEditSkills to patch newData directly. Then pass that down. This will greatly reduce the surface area for bugs and remove unnecessary effects in SkillEdit.set[Something]. Generally, in react, you want to keep to the naming convention of user triggered events beginning with on. The trouble with the former, is it implies that the user action does a certain thing, which makes the component look like it's less reusable in other contexts (even though it's the same behaviour really, it's just incorrect naming).setState (which is passed the current value) instead of referencing the state value returned from useState. This avoids bugs to do with accidentally getting a stale value when you update state in a loop or something. React doesn't actually update the value of the state immediately, it gets flushed at the end of the call stack. Using the previous value callback gets around this. Not a problem that will show up in your code, just good practice that you should get in the habit of.show to the effect that resets the newData state. If you really wanted it how it was though you can just remove that again from the deps array.data prop which is not allowed in react. I've changed to Object.assign to make sure it's a proper edited clone. The way this works if everything gets merged from right to left. So by the first param being {}, you start with a brand new object and then you load int the old + new data into it. Libraries like Immer make this easier, you might want to look into it.data state null and then only populate it when the request comes back like you do now. Then in the top level render you'd check for null and return some loading display instead of ModalEditSkills. Similar stuff would happen if the network errored. You might also want to have some new state that says if an error happened (instead of just logging), and check that as well and display the error page.Here's the code with my proposed changes (minus the library change, you'd still need to do that if you cared enough).
And here's a code sandbox with it working: https://codesandbox.io/s/wispy-meadow-3ru2nq?file=/src/App.js (I replaced the network call with static data for testing, and I don't have your icons).
const EditPerson = () => {
const [data, setData] = useState({skills: []});
const { id } = useParams();
useEffect(() => {
const fetchData = async () => {
const docRef = doc(db, "person", id);
try {
const docSnap = await getDoc(docRef);
setData(docSnap.data());
} catch (error) {
console.log(error);
}
};
fetchData().catch(console.error);
}, []);
const handleSkillsChanged = (skills) => {
setData(data => ({...data, skills}));
}
return (
<div>
<ModalEditSkills
onSkillsChanged={handleSkillsChanged}
data={data.skills}
/>
</div>
);
}
const ModalEditSkills = ({ data, onSkillsChanged}) => {
const [show, setShow] = useState(false);
const [newData, setNewData] = useState([]);
useEffect(() => {
setNewData(data);
}, [data, show]);
const handleClose = () => setShow(false);
const handleShow = () => setShow(true);
const handleSkillChange = (index, name, value) => {
setNewData(prevValues => {
const newValues = [...prevValues]
newValues[index] = Object.assign({}, newValues[index], { [name]: value });
return newValues
});
}
const handleSkillAdded = () => {
setNewData(prevValues => [...prevValues, { skill: "", stars: 0 }]);
}
const handleSkillRemoved = (index) => {
setNewData(prevValues => {
const newValues = [...prevValues];
newValues.splice(index, 1);
return newValues
});
}
const handleSubmitSkills = (e) => {
e.preventDefault();
onSkillsChanged(newData);
setShow(false);
};
return (
<>
<div className="content_header">
<div className="content_header_top">
<div className="header_left">Skills</div>
<div className="header_right">
<Button className="round-btn" onClick={handleShow}>
<i className="fa-solid fa-pencil t-14"></i>
</Button>
</div>
</div>
</div>
{show && (
<Modal show={show} onHide={handleClose} size="">
<Modal.Header closeButton>
<Modal.Title>Edit Person</Modal.Title>
</Modal.Header>
<Modal.Body>
<SkillEdit
data={newData}
onSkillChanged={handleSkillChange}
onSkillAdded={handleSkillAdded}
onSkillRemoved={handleSkillRemoved}
/>
</Modal.Body>
<Modal.Footer>
<Form>
<Button
className="btn-skill-complete"
onClick={handleSubmitSkills}
>
Save
</Button>
</Form>
</Modal.Footer>
</Modal>
)}
</>
);
};
const SkillEdit = ({ data, onSkillChanged, onSkillRemoved, onSkillAdded}) => {
return (
<Form>
<div>
{data?.map((inputField, index) => (
<div key={index}>
<Row>
<Col xs={5} md={5}>
<Form.Group as={Col}>
<Form.Control
className="mb-3"
type="text"
id="skill"
name="skill"
value={inputField?.skill}
onChange={(event) =>
onSkillChanged(index, "skill", event.target.value)
}
/>
</Form.Group>
</Col>
<Col xs={4} md={4}>
<div>
<Form.Group>
<ReactStars
type="number"
name="stars"
count={5}
size={24}
id="stars"
onChange={(newValue) =>
onSkillChanged(index, "stars", newValue)
}}
emptyIcon={<i className="fa-solid fa-star"></i>}
filledIcon={<i className="fa-solid fa-star"></i>}
value={inputField.stars}
/>
</Form.Group>
</div>
</Col>
<Col xs={3} md={3}>
<div>
<button
type="button"
onClick={onSkillAdded}
>
<i className="fa-solid fa-plus"></i>
</button>
<button
type="button"
onClick={() => onSkillRemoved(index)}
>
<i className="fa-solid fa-minus"></i>
</button>
</div>
</Col>
</Row>
</div>
))}
</div>
</Form>
);
};