-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[최준엽] week11 #369
The head ref may contain hidden characters: "part2-\uCD5C\uC900\uC5FD"
[최준엽] week11 #369
Conversation
yup299
commented
Apr 30, 2024
수고 하셨습니다 ! 위클리 미션 하시느라 정말 수고 많으셨어요. |
저번 10주차 코드를 미완성하고나니 예제 코드를 보고 하려하니 코드를 읽는데 너무 오래 걸려서 아예 거의 진행을 하지못했습니다 차후 시간이 나는대로 조금식 해당 기능을 구현해보겠습니다.넵넵. 좋습니다 ! 이번 모달창을 만들면서 말씀해주신 리액트 포탈 기능에대해 처음 들어보게되었는데 DOM트리상의 위치를 변경해주는 기능이라고 조사해보았습니다 그런데 이거를 어떤 느낌으로 써야될지 잘 모르겠어서 혹시 관련 참고할만한게있으면 참고해보겠습니다넵넵 ! 제가 본 것 중에서 잘 정리되어 있는 블로그 첨부드리겠습니다. |
템플릿 코드 외에 준엽님께서 커밋하신 내역을 기준으로 코드리뷰 진행하도록 하겠습니다 😊 |
@@ -0,0 +1,18 @@ | |||
import * as S from './Modal.styled.js'; | |||
|
|||
function Modal() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공통 모달 컴포넌트를 만드셨군요 !
해당 모달을 사용하여 각 목적에 맞는 모달들을 만드려고 하시는걸까요? 😊
<S.InputWrapper> | ||
<S.InputBox type="text" /> | ||
<S.Button>변경하기</S.Button> | ||
</S.InputWrapper> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 컴포넌트는 공통 모달 컴포넌트로 보입니다.
해당 기능은 공통 모달에 넣기에는 목적이 뚜렷한 것으로 보여요 ! children
으로 분리해서 부모가 모달의 기능을 결정지을 수 있게 구조를 바꿔보면 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
또한, 다음과 같이 useModal
을 만들어 보는 것도 좋은 방법입니다(준엽님께서 질문주셨던 useReducer
가 포함되어있습니다 😊):
import React, { useContext, useReducer } from 'react';
const ModalContext = React.createContext();
const modalReducer = (state, action) => {
switch (action.type) {
case 'OPEN_MODAL':
return { ...state, [action.modalName]: true };
case 'CLOSE_MODAL':
return { ...state, [action.modalName]: false };
default:
throw new Error(`Unhandled action type: ${action.type}`);
}
};
export const ModalProvider = ({ children }) => {
const [modalState, dispatch] = useReducer(modalReducer, {});
const openModal = (modalName) => {
dispatch({ type: 'OPEN_MODAL', modalName });
};
const closeModal = (modalName) => {
dispatch({ type: 'CLOSE_MODAL', modalName });
};
return (
<ModalContext.Provider value={{ modalState, openModal, closeModal }}>
{children}
</ModalContext.Provider>
);
};
export const useModal = () => useContext(ModalContext);
@@ -0,0 +1,9 @@ | |||
import ReactDom from 'react-dom'; | |||
|
|||
const ModalPortal = ({ children }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
포탈을 따로 분리하셨군요. 🤔
모달 안에 포탈 로직을 넣는 것 vs 모달 포탈을 따로 구분하는 것
고민이 될 수 있을 것 같아요. 준엽님은 따로 포탈을 분리하신 이유가 있으실까요?
바쁘신 와중에 수고 정말 많으셨습니다 준엽님. |
리뷰 중 궁금하신 점 있으시면 사전 질문을 통해서 편하게 질문주세요 ~! |