Theo's Typesafe Cult•8mo ago

Appropriate Use of Modals - Attach to mapped components or pass modal logic down?

Is there considerable downside to attaching a modal to individual mapped components? Is it better/preferred to have a centralized Modal on the page and then pass down modal logic to the mapped components? Example 1: Modal on each mapped component

const ToDoComponent = ({ todo }) => {
  // logic for modal here
  return (
    <div>
      {todo}
      <ModalThatDoesSomething />
    </div>
  );
};

const Page = () => {
  const todos = [todo1, todo2, todo3, ...];

  return <div>
    {todos.map((todo) => (
      <ToDoComponent todo={todo} />
    )}
  </div>;
};

export default Page;

const ToDoComponent = ({ todo }) => {
  // logic for modal here
  return (
    <div>
      {todo}
      <ModalThatDoesSomething />
    </div>
  );
};

const Page = () => {
  const todos = [todo1, todo2, todo3, ...];

  return <div>
    {todos.map((todo) => (
      <ToDoComponent todo={todo} />
    )}
  </div>;
};

export default Page;

Example 2: Centralized Modal

const ToDoComponent = ({ todo, modalLogic }) => {
  // modal logic is triggered and passed up to parent
  return (
    <div>
      {todo}
    </div>
  );
};

const Page = () => {
  const todos = [todo1, todo2, todo3, ...];
  // MODAL LOGIC HERE


  return <div>
    {todos.map((todo) => (
      <ToDoComponent todo={todo} modalLogic={modalLogic} />
    )}

    <ModalThatDoesSomething />
  </div>;
};

export default Page;

const ToDoComponent = ({ todo, modalLogic }) => {
  // modal logic is triggered and passed up to parent
  return (
    <div>
      {todo}
    </div>
  );
};

const Page = () => {
  const todos = [todo1, todo2, todo3, ...];
  // MODAL LOGIC HERE


  return <div>
    {todos.map((todo) => (
      <ToDoComponent todo={todo} modalLogic={modalLogic} />
    )}

    <ModalThatDoesSomething />
  </div>;
};

export default Page;

6 Replies

Mo3geza•8mo ago

So here in the centralized modal example you wrote your logic in the page component that renders each Todo, so if modal logic contains a useState or anything could cause any re-renders the app component gonna render the todos again so if you have just one task you need to render the model in the app going to re-render all todos, which is bad for performance, but if you used the modal on each mapped component example if you triggered the modal the the only thing that is gonna re-render is the only task which triggered the modal, so no need for unnecessary re-rendering of the other todos in the centralized modal example

michaeldrotar•8mo ago

If your modal is loading or doing anything before it's opened then there's also the cost of doing that N times vs 1 time. It may be helpful to think of what the "modalLogic" is. If it's onClick={() => openDetailsModal(todo)} then having a parent modal might be a clear abstraction. If it's passing modal-specific functions to the ToDoComponent then that's probably a less sensible coupling. It might help to think about if you changed it from opening a modal to going to a new page or sliding out a drawer if your component's props would still make sense. If you're using Next.js then you might find parallel and intercepting routes helpful. It sounds like a case of trying to open a detail pane or edit form for the todo item.

CircusOP•8mo ago

@michaeldrotar Correct me if I'm wrong, but it sounds like if there's any kind of complexity, then to keep the modal on the component? So if the modal handled deleting that specific ToDo from a ToDo list or something more, just keep the modal on each respective component?

michaeldrotar•8mo ago

I wouldn't say that, no. "Any kind of complexity" is very vague. Your reference to modalLogic implied a coupling of data and presentation that I was trying to warn about, and I'm guessing that you're doing a typical To-do list with CRUD operations. I can give a fuller example:

// Bad Example (couples business logic with presentation)
function Todos() {
  const [todos, setTodos] = useState(() => [ /* some data */ ])
  return {todos.map((todo) => <TodoItem todo={todo} todos={todos} setTodos={setTodos} />}
}

function TodoItem({ todo, todos, setTodos }) {
  return <div>{todo.label} <button onClick={() => setTodos((todos) => todos.filter((currTodo) => currTodo !== todo))}>Delete</button></div>
}

// Bad Example (couples business logic with presentation)
function Todos() {
  const [todos, setTodos] = useState(() => [ /* some data */ ])
  return {todos.map((todo) => <TodoItem todo={todo} todos={todos} setTodos={setTodos} />}
}

function TodoItem({ todo, todos, setTodos }) {
  return <div>{todo.label} <button onClick={() => setTodos((todos) => todos.filter((currTodo) => currTodo !== todo))}>Delete</button></div>
}

In the "Bad Example", the child TodoItem is able to modify the parent Todos data. It creates a complex data flow and tightly couples the component to the data. DDAU (Data Down, Actions Up) is an idea that data should only flow down into children, and children should only emit actions up to the parent.

// Good example (DDAU, separate data vs presentation)
function Todos() {
  const [todos, setTodos] = useState(() => [ /* some data */ ])
  return {todos.map((todo) => <ListItem label={todo.label} onDeleteClick={() => setTodos((todos) => todos.filter((currTodo) => currTodo !== todo))} />}
}

function ListItem({ label, onDeleteClick }) {
  return <div>{label} {onDeleteClick && <button onClick={onDeleteClick}>Delete</button>}</div>
}

// Good example (DDAU, separate data vs presentation)
function Todos() {
  const [todos, setTodos] = useState(() => [ /* some data */ ])
  return {todos.map((todo) => <ListItem label={todo.label} onDeleteClick={() => setTodos((todos) => todos.filter((currTodo) => currTodo !== todo))} />}
}

function ListItem({ label, onDeleteClick }) {
  return <div>{label} {onDeleteClick && <button onClick={onDeleteClick}>Delete</button>}</div>
}

In this example, Todos owns everything to do with the data. And ListItem is reusable. The same pattern works for your modal. The child ListItem could emit an onEditClick or onOpenClick or whatever the action is. Then have the parent open the modal.

CircusOP•8mo ago

Perhaps I steered the conversation incorrectly by trying to go with a simple ToDo list, apologies. As of right now, I have the parent server component that fetches the data. Each child component has access to the database and can manipulate its data correctly in the database. Each child component has the modal component that is hidden, and the modal component is all that uses state. I think what you're saying is move the modal logic up, but that would make a server component into a client component?

michaeldrotar•8mo ago

Hm let's rewind a bit. I think I'm making bad assumptions. 😄 Sticking to the questions...

Is there considerable downside to attaching a modal to individual mapped components?

Probably. But there are factors to consider. If you're mapping over 10 components then it's probably fine to have 10 modals. But not if they're huge. Or not if you have 1000 of them. Sites slow down when the DOM gets large so I would look at how big/complex the modal component is. I would expect that having a single modal would be the simpler solution. But if there are compelling reasons to have one modal per item, things like lazy loading and memoizing can help maintain performance.

Is it better/preferred to have a centralized Modal on the page and then pass down modal logic to the mapped components?

It's perfectly fine to do something like setModalTodo(todo); setModalOpen(true) and have <TodoModal todo={modalTodo} open={modalOpen} /> - is that what you're asking?

I think what you're saying is move the modal logic up, but that would make a server component into a client component?

If you do go with a single modal, I would still keep the server component too.

export async function Page() { // <-- server component
  const todos = await getTodos()
  return <Todos todos={todos} />
}

export function Todos({ todos }) { // <-- client component
  const [modalTodo, setModalTodo] = useState()
  const [modalOpen, setModalOpen] = useState(false)
  return <>
    {todos.map((todo) => <TodoItem ... /> )}
    <TodoModal todo={modalTodo} open={modalOpen} />
  </>
}

export function Todo({ ... }) { // <-- client component
  return <>...</>
}

export async function Page() { // <-- server component
  const todos = await getTodos()
  return <Todos todos={todos} />
}

export function Todos({ todos }) { // <-- client component
  const [modalTodo, setModalTodo] = useState()
  const [modalOpen, setModalOpen] = useState(false)
  return <>
    {todos.map((todo) => <TodoItem ... /> )}
    <TodoModal todo={modalTodo} open={modalOpen} />
  </>
}

export function Todo({ ... }) { // <-- client component
  return <>...</>
}

Gaming

Programming

Appropriate Use of Modals - Attach to mapped components or pass modal logic down?