1

The site has a button for deleting records (DeleteForeverIcon in the code). When you click on this button, a window opens (made according to the documentation using Dialog Mui).

When you click on the "Confirm" button, the handleDeleteItem() function is launched, which deletes the entry. But I can’t understand why the window closes while this function is running, because I don’t switch the state anywhere

Looking for a solution on my own, I added console.log() to my code (below is the same code, only with console.log()) and came up with the following conclusion: when I run the handleDeleteItem() function, the open state switches to false and so the window closes. But why does it switch to false?

export function DeleteButton({ item }) {
  const { urlParams } = useContext(PageContext)
  const { firestore } = useContext(AppContext)
  const [open, setOpen] = useState(false);

  console.log(open, "window close")                    // console.log here
  const handleClickOpen = () => {
    setOpen(true);
  };

  const handleClose = () => {
    setOpen(false);
  };

  const handleDeleteItem = async () => {
    console.log("start")                         // console.log here
    await deleteItem()
    console.log("finish")                        // console.log here
  }

  return (
      <ButtonGroup>
          <DeleteForeverIcon onClick={handleClickOpen}/>
    
          <Dialog
              open={open}
              onClose={handleClose}>
              
              <DialogActions>
                <Button onClick={handleClose}>Cancel</Button>
                <Button onClick={handleDeleteItem}>Confirm</Button>
              </DialogActions>

            </Dialog>
            
        </ButtonGroup >
  )
}

enter image description here

That is, summing up the question, I would like the window to close only after the deleteItem () function has completed, and not during its execution

Paul
  • 53
  • 3
  • 21
  • 1
    Are you able to create a code sandbox or a stack snippet reproducing this issue so that we can debug it for ourselves (see [mre]) – Nick Parsons Oct 21 '22 at 08:48
  • @Nick Parsons I can create, but I have no idea how to put the deleteItem () function there, since it is connected with my database (firestore). Can you advise something? – Paul Oct 21 '22 at 09:11
  • 1
    @Paul That's okay, you can include a dummy promise instead of it :) – KcH Oct 21 '22 at 09:27
  • @Paul Yeah, as KcH suggests you can use a dummy promise, eg: `await sleep()` (you can look at how `sleep()` is implemented here: [What is the JavaScript version of sleep()?](https://stackoverflow.com/a/39914235)) – Nick Parsons Oct 21 '22 at 09:29
  • @NickParsons Instead of await deleteItem() , I added await new Promise((resolve) => setTimeout(resolve, 3000)); It's amazing, but everything works correctly. So this is a problem in await deleteItem() ? – Paul Oct 21 '22 at 09:41
  • @Paul It could be. What is `deleteItem()` doing? To me it looks like your component is being rerendered after you call `handleDeleteItem()` (which could happen being the parent is rerendering, or you're updating the state of `DeleteButton` somewhere. – Nick Parsons Oct 21 '22 at 09:59
  • If you add a log to the component that's rendering ``, do you also see logs occurring in that before the `"finish"` is logged? – Nick Parsons Oct 21 '22 at 10:30
  • @Nick Parsons I'm sorry I didn't understand your question – Paul Oct 21 '22 at 10:40
  • @Paul somewhere in your real code you would be using the `` component (most likely within another component). If you add a `console.log()` to that component so that you can see when it gets rerendered, do you see that log appear between the `start` and `finish` logs that you're getting? – Nick Parsons Oct 21 '22 at 11:04
  • @Paul So in your code, if you were to add a console.log to `DevicesTableCell` before the `return`, do you also see that execute between the `start` and `finish` logs? – Nick Parsons Oct 21 '22 at 11:40
  • @Nick Parsons Yes, if I add console.log("intermediate text") before the return in the DevicesTableCell, then when I run handleDeleteItem, between start and finish, I see the inscription "intermediate text" in the console – Paul Oct 21 '22 at 11:50
  • Ok, that hints that calling `handleDeleteItem` is causing your state to update somehow, which is causing your components to rerender (maybe you have some sort of listener set up that reacts to changes in your db and updates state higher up in your component tree?). Since you have multiple `DeleteButton`s being rerendered, each will have its own log, so the `false` that you're seeing for the state value may be from a different instance from within a different `DevicesTableCell` component, and not your state-changing (that's just my guess). It's a bit hard to tell without being able repro it – Nick Parsons Oct 21 '22 at 11:59
  • @Nick Parsons I understood you. Thanks for the advice. I will look for reasons – Paul Oct 21 '22 at 12:58
  • @Paul Even if it's rerendering, it shouldn't be making your dialog window close, because the `open` state value within your `DeleteButton` should remain as `true` even after a rerender. The only case I can think of this potentially changing is if you're rendering `DeleteButton` with a `key` prop that isn't remaining consistent throughout rerenders. In your codesanbox it doesn't look like you're doing that. In your real code, are you using `.map()` inside of either `DevicesTable` or `DevicesTableCell` to create your JSX and using a `key` prop? – Nick Parsons Oct 24 '22 at 09:46
  • @Nick Parsons Yes, you're right, in real code I use map() in DevicesTable (I just added it for you as a visual example) – Paul Oct 24 '22 at 09:50
  • @Paul cool, no worries, are you able to provide an example on how you're mapping your elements? – Nick Parsons Oct 24 '22 at 09:52
  • @Nick Parsons Sorry, I added slightly wrong code in the last change, but I already fixed it. – Paul Oct 24 '22 at 09:58
  • @Nick Parsons Yes, I can give an example, but I did not quite understand what exactly you want to see – Paul Oct 24 '22 at 09:58
  • @Paul It's ok, actually, I think I see what is wrong now. While you haven't shown it in your code, it seems like you have an `onSnapshot` listener somewhere in your code that updates your state in `DevicesTable` whenever your database rows change. When you delete a record using your button, the database updates, which then triggers the `onSnapshot` listener to update your state and cause your component to rerender the list within your table. Because you deleted the record, the record you previously rendered won't be rendered again, and so the `` associated with it won't display. – Nick Parsons Oct 24 '22 at 10:01
  • So essentially, you need to move the `` up to your `DevicesTable` component which will always be rendered, unlike the invidual rows/records you're rendering which can be deleted and thus all the contents within it will be removed (including the dialog) – Nick Parsons Oct 24 '22 at 10:03
  • @Nick Parsons Yes, it is, I use onSnapshot (I just didn't think that I would have to dig so deep to solve this problem). If necessary, I will give an example code with onSnapshot. I need it, because, as you understand, I work with a firestore, and everything should be displayed in real time. – Paul Oct 24 '22 at 10:29
  • @Nick Parsons Could you show me how to properly move the to the DevicesTable component? – Paul Oct 24 '22 at 10:30

1 Answers1

1

Based on some further clarification in the comments, it seems as though your issue is to do with the fact that calling deleteItem(...) causes your state to update in your parent components (due to an onSnapshot firebase listener). Your parent components are responsible for rendering the children components. When your state updates, the item/row that you deleted won't be in the new state value, and so the component that was rendering your Dialog previously won't be rendered (because it has been deleted).

Here is a minified example of your issue:

const { useState } = React;
const List = () => {
  const [items, setItems] = useState(["a", "b", "c", "d", "e"]);
  
  const handleDelete = (charToDel) => {
    setItems(items => items.filter(char => char !== charToDel));
  }
  
  return <ul>
    {items.map(char =>
      <li key={char}>{char} - <DeleteButton value={char} onDelete={handleDelete}/></li>
    )}
  </ul>
}

const DeleteButton = ({value, onDelete}) => {
  const [open, setOpen] = useState(false);
  return <React.Fragment>
    <button onClick={() => setOpen(true)}>&times;</button>
    
    <dialog open={open}>
      <p>Delete {value}?</p>
      <button onClick={() => onDelete(value)}>Confirm</button>
    </dialog>
  </React.Fragment>
}

ReactDOM.createRoot(document.body).render(<List />);
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/18.2.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/18.2.0/umd/react-dom.production.min.js"></script>

So since you're not rendering the component that renders the Dialog once you remove the item, you won't be rendering the <Dialog> anymore and so it disappears.

One way to fix this is to lift the <Dialog> component up to a component that doesn't get removed when you remove an item from your state. By the looks of things, the closest parent component that has this property is DevicesTable. In there you can render your dialog and keep track of a selectedItem to determine which item that should be deleted, which you can set based on the item you press (see code comments below):

// DevicesTable component

const [selectedItem, setSelectedItem] = useState();

const handleClose = () => {
  setSelectedItem(null);
}

const handleDeleteItem = () => { // this doesn't need to be `async` if not using `await`
  deleteItem(selectedItem, firestore, urlParams); // this doesn't need to be `await`ed if you don't have any code following it
}

return (
    <>
      {/* vvvvvv -- move dialog here -- vvvvvv */}
      <Dialog open={!!selectedItem} onClose={handleClose}>
        <DialogActions>
          <Button onClick={handleClose}>Cancel</Button>
          <Button onClick={handleDeleteItem}>Confirm</Button>
        </DialogActions>
      </Dialog>
      {/* ^^^^^^ -- move dialog here -- ^^^^^^ */}
      <TableContainer className="TableContainerGridStyle">
        <Table className="TableStyle">
          <DevicesTableHeader />
          <TableBody className="TableBodyStyle">
            {devices.map((device) =>
              <DevicesTableCell 
                device={device}
                onDeleteButtonPress={() => setSelectedItem(device)} /* <--- set the selected item */
                key={device.description.id}
              />
            )}
          </TableBody>
        </Table>
      </TableContainer>
    </>
);

For brevity, I've removed the open state and instead used the presence of the selectedItem to determine if the modal should be open or not, but you can of course add that back in if you wish and set both the selectedItem and the open state when opening and closing the modal.

Within DevicesTableCell, you would then grab the onDeleteButtonPress prop, and then pass it to DeleteButton like so:

//                                   v-- grab the function
function DevicesTableCell({ device, onDeleteButtonPress }) {
  ...
  <DeleteButton item={device} onDeleteButtonPress={onDeleteButtonPress}/> {/* pass it to the componennt */}
  ...
}

Within DeleteButton you should then invoke the onDeleteButtonPress function:

<DeleteForeverIcon onClick={onDeleteButtonPress} />

If you don't like the idea of passing callbacks down through multiple components like this, you can avoid that by using useReducer with a context, as described here.

Nick Parsons
  • 45,728
  • 6
  • 46
  • 64
  • Cool! I'm sure it will work, but I need some time to understand your idea. I think I'll try to figure it out by the end of the day and get back to you. Could you show me what I should specifically change in DevicesTableCell? – Paul Oct 24 '22 at 11:32
  • I think I understood what you meant about DevicesTableCell. – Paul Oct 24 '22 at 11:45
  • @Paul yes, it's the same idea as what you've done with `device` but now also with the `onDeleteButtonPress` function/prop. I added an edit now – Nick Parsons Oct 24 '22 at 11:45
  • 1
    Yes, everything works great! Thank you very much for helping to find the root of the problem and help to solve it. You are a wonderful person and a great programmer – Paul Oct 24 '22 at 13:27