Is this a good practice for resetting states on dialog close

[mattp0123]

Let's say I have a dialog like this:

import * as Dialog from '@radix-ui/react-dialog'
import * as Styled from './styled'
import useDataFromAPI from 'hooks/use-data-from-api'

type DialogProps = { data: string; saveChanges: (data: string) => void; }

function MyDialog(props: DialogProps) {
  // A draft state that should be cleaned up on dialog close if save button is not clicked.
  const [input, setInput] = useState(props.data)
  useEffect(() => setInput(props.data), [props.data])

  // Call this function on Dialog.Close click
  const handleResetStates = () => setInput(props.data)
  return (
    <Dialog.Root>
      <Dialog.Trigger>Open dialog</Dialog.Trigger>
      <Styled.Content>
        <input
          value={input}
          onChange={(event) => setInput(event.target.value)}
        />
        <Dialog.Close onClick={() => props.saveChanges(input)}>
          Save
        </Dialog.Close>
        <Dialog.Close onClick={handleResetStates}>Close</Dialog.Close>
      </Styled.Content>
    </Dialog.Root>
  )
}

function App() {
  const data = useDataFromAPI()
  return (
    <>
      <MyDialog data={data} />
    </>
  )
}

Should I reset the dialog states like above, or:

type DialogProps = {
  data: string;
  saveChanges: (data: string) => void;
  isOpen: boolean;
  setIsOpen: (isOpen: boolean) => void;
}

function MyDialog(props: DialogProps) {
  const [input, setInput] = useState(props.data)
  useEffect(() => setInput(props.data), [props.data])

  const handleResetStates = useCallback(
    () => setInput(props.data), [props.data]
  )
  useEffect(handleResetStates, [props.isOpen, handleResetStates])
  
  return (
    <Dialog.Root open={props.isOpen} onOpenChange={props.setIsOpen}>
      <Dialog.Trigger>Open dialog</Dialog.Trigger>
      <Styled.Content>
        <input
          value={input}
          onChange={(event) => setInput(event.target.value)}
        />
        <Dialog.Close onClick={() => props.saveChanges(input)}>
          Save
        </Dialog.Close>
        <Dialog.Close onClick={handleResetStates}>Close</Dialog.Close>
      </Styled.Content>
    </Dialog.Root>
  )
}

[mattp0123]

I ended up doing like this, so I don't have to worry about resetting dialog states, mounting & unmounting will do the job. (Still wondering if this is a good way.):

function App() {
  const [data, mutateData] = useDataFromAPI()
  const dialogToggle = useToggle()
  return (
    <div className="dialog-trigger-attachable" style={{ position: 'relative' }}>
      <button className="real-dialog-trigger" onClick={dialogToggle.on}>
        Open dialog
      </button>
      {dialogToggle.isOn && (
        <MyDialog
          data={data}
          setIsOpen={dialogToggle.set}
          saveChanges={mutateData}
        />
      )}
    </div>
  )
}

type DialogProps = {
  data: string;
  saveChanges: (data: string) => void;
  setIsOpen: (isOpen: boolean) => void;
}

function MyDialog(props: DialogProps) {
  const [input, setInput] = useState(props.data)
  return (
    // The open state will always be true
    // because this dialog will unmount if dialogToggle.isOn is set to false
    <Dialog.Root open={true} setIsOpen={props.setIsOpen}>
      {/* The trigger is only used as the positioning anchor for content */}
      <Dialog.Trigger asChild>
        <div style={{ position: 'absolute', inset: 0, pointerEvents: 'none' }} />
      </Dialog.Trigger>
      <Styled.Content>
        <input
          value={input}
          onChange={(event) => setInput(event.target.value)}
        />
        <Dialog.Close onClick={() => props.saveChanges(input)}>
          Save
        </Dialog.Close>
        <Dialog.Close>Cancel</Dialog.Close>
      </Styled.Content>
    </Dialog.Root>
  )
}

[benoitgrelard]

I'm wondering why you are bypassing Dialog.Trigger? It is an important part of the pattern and you are most likely breaking accessibility by doing so.

If you can create a codesandbox with what you are trying to achieve, it might be easier for us to help rather than trying to interpret written code.

✌️

[mattp0123]

@benoitgrelard Thanks for replying. Congrats on v1.0.0. 🎉
https://codesandbox.io/s/radix-popover-unmountable-8vicq9
As this example, I want the input state inside popover component to be cleaned up (reset to initial value) when the popover is closed.

[jjenzz]

it will automatically reset if you use defaultValue on the input instead (uncontrolled pattern): https://codesandbox.io/s/icy-river-08660t?file=/src/App.js

[sagar-niural]

@jjenzz what if i dont want to reset? Is there any control that is given in antd modals like destroyOnClose. Where i can control whether to unmount the children when dialog is closed?

[columk1]

Also interested to know if there is a recommended pattern for this. I would like to use a Trigger, but found it messy to reset state on close. I chose to enable unmounting by wrapping the Dialog. I can pass a trigger to this wrapper component but then I'm not using the Radix trigger.