Skip to content
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

✨Feat : 세팅 사이드 바 UI / 로직 #50

Merged
merged 15 commits into from
Sep 22, 2023
Merged

Conversation

hyesung99
Copy link
Collaborator

🪄 변경사항

  1. index.html에 modal-root라는 div를 만들고 react.createPortal을 통해 사이드바를 트리 구조 최상위로 연결했습니다.
    따라서 컴포넌트가 화면을 덮는 로직을 쉽게 구현할 수 있습니다 (z-index 꼬이지 않음)

  2. profile.tsx에서 sidebarVisible의 상태를 컨트롤 하여 사이드바를 보여줄 지 판단합니다.

  3. 프로필 수정 클릭
    주소에 #edit 해시를 주입하여 profile.tsx에서 recoil상태를 통해 현재 editmode인지 판단하게 만듭니다.

  4. 비밀번호 변경 클릭 => 이미 만들어져 있는 비밀번호 변경 페이지에 진입

  5. 로그아웃 클릭 => 세션스토리지의 유저 정보를 삭제하고 새로고침하여 로그인 페이지로 redirect해줍니다.

🖥 결과 화면

image

✏️ PR 포인트

  1. 화면을 덮는 사이드바를 위해 modal-root를 트리구조 가장 상위에 선언했습니다 (index.html). 이해하기 힘든 구조인지 궁급합니다.

  2. 사이드바를 보여줄지 판단하는 선택권을 profile.tsx가 가지고 결과에 따라 렌더링 하고있는데 사이드 바 스스로 자신의 컴포넌트를 보여줄지 결정해야 하는지 고민이 됩니다.

@hyesung99 hyesung99 self-assigned this Sep 18, 2023
Copy link
Collaborator

@nayeon-hub nayeon-hub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM👍
modal-root가 <div id="root"></div>의 하위로 들어가지 않는 이유가 궁금합니다! (portal 잘 모름..!)
index.html에 있다는 것을 예상하지 못하면 잘 찾지 못할 것 같기는 한데, 다른 방법도 있나요?

modal 스스로 보여줄지 말지를 판단하려면 기준이 있어야 되는데 어떤 기준이 있나요?


const Profile = () => {
const { userId } = useParams<{ userId: string }>();
const location = useLocation();
const [sidebarVisible, setSidebarVisible] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

visible 을 모달에서도 쓰고, 검색에서도 쓰일 것 같은데 전역 커스텀 훅으로 뺄까요? openSidebar, closeSidebar 함수랑 같이요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 방법인 것 같아요!

Comment on lines 30 to 36
const openSidebar = () => {
setSidebarVisible(true);
};

const closeSidebar = () => {
setSidebarVisible(false);
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const openSidebar = () => {
setSidebarVisible(true);
};
const closeSidebar = () => {
setSidebarVisible(false);
};
const showSidebar = () => {
setSidebarVisible(prev => !prev);
};

이렇게 하나로 묶을 수도 있습니다!


const SettingSideBar = () => {
return (
<SettingSideBarBackground>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p5

까만 배경화면을 Deem 이라고 쓴다고 강의해서 하던데 혹시 이 단어를 사용해서 변수명을 짓는 건 어떠실까요?!

@@ -30,7 +30,7 @@ const Profile = () => {

return (
<ProfilePage>
{editMode && <SettingSideBar />}
<SettingSideBar active={editMode ? true : false} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2

editMode 도 boolean 타입의 값인 거 같은데 editMode 자체를 넣어주거나, SettingSideBar 에서 editMode 를 호출하는 것도 좋아보여요!

}

const SettingSideBar = ({ active }: SettingSideBarProps) => {
const setEditMode = useSetRecoilState(editModeState);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p2

SettingSideBar 에서도 useSetRecoilState 를 불러온다면, props 로 editmode 에 해당하는 값을 받아올 필요가 없지 않을까요!

Comment on lines 60 to 63
</SettingRightSideBar>
</SettingSideBarSection>,
document.getElementById('modal-root')
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CreatePortal 사용법 잘 알아갑니다 혜성님! 👍

Comment on lines 44 to 47
> a {
text-decoration: none;
color: ${({ theme }) => theme.color.black};
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link 컴포넌트의 색상 설정 css 가 잘못되어 있어서 아마 적용이 안 된 거 같아요! posts 페이지를 작성하면서 수정해놓았으니 해당 구문은 없애도 될 거 같습니다!

@hyesung99 hyesung99 merged commit bda4943 into main Sep 22, 2023
@hyesung99 hyesung99 deleted the feat/profile/settings branch September 23, 2023 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants