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

Progress 페이지에서 Sidebar가 Header 위에 쌓이는 현상 #167

Merged
merged 4 commits into from
Jan 3, 2025

Conversation

Sunjae95
Copy link
Contributor

@Sunjae95 Sunjae95 commented Dec 28, 2024

#125 (comment) 이 맥락을 보고 z-index를 사용하지 않으려 했지만 별도의 방법이나 저희가 원하는 UI reference를 참고하지 못하여 떠오르지 z-index를 사용했습니다.
z-index가 전역으로 미치는것을 고려한다면 각각의 z-index를 설정하는것보단 z-index값을 root에서 관리하는건 어떨까하여 코드를 수정해서 올려봅니다.

2024-12-29.12.56.51.mov

추가로 sidebar관련 스타일 불필요하거나 수정되어야하는 부분을 수정했습니다.

체크리스트

  • 이슈가 연결되어 있나요?
  • 배포 후 브라우저 콘솔에 경고나 오류가 있나요?

@DaleSeo
Copy link
Contributor

DaleSeo commented Dec 30, 2024

fixes #173

(우려하셨던데로 이슈가 올라왔네요 ㅋㅋ 😝)

Copy link
Contributor

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

z-index를 통해 문제를 해결한 부분은 좀 아쉽지만 😢 z-index의 값을 CSS 변수화 하신 것은 좋은 접근인 것 같습니다 💯

@@ -8,6 +8,11 @@ const meta = {
disable: true,
},
},
decorators: (Story: StoryFn) => (
<div style={{ width: "203px" }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

이 건 왜 필요한 거죠?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

progress 컴포넌트에서 sidebar 너비를 지정해주고 있어서 우리가 알던 사이드바와 다르다고 판단될 수 있어서 부모에서 너비를 지정해주도록 decorators를 사용했어요.
지정해주는 dom까지 sidebar 컴포넌트에 포함하려는 생각도 있었지만 이전 작업자의 의도를 따라 sidebar의 스타일만 수정했습니다!

Copy link
Contributor

@DaleSeo DaleSeo Jan 3, 2025

Choose a reason for hiding this comment

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

아하! 설명 감사합니다. 그럼 차라리 사이드 컴포넌트에 max-width를 주는 건 어떠세요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 그러면 decorators를 사용할 필요가 없어지겠네요
반영해서 수정할게요!
의견 감사합니다 😊

@DaleSeo
Copy link
Contributor

DaleSeo commented Dec 30, 2024

헤더와 사이드바 사이에 완전히 딱 붙지 않고 공간이 쪼끔 있으면 더 나을 것 같은데 어떻게 생각하세요? @yolophg

Shot 2024-12-30 at 14 14 02

@Sunjae95 Sunjae95 force-pushed the 152-sidebar-header-fix branch from 18715c8 to 5f04786 Compare January 3, 2025 12:07
@Sunjae95
Copy link
Contributor Author

Sunjae95 commented Jan 3, 2025

#167 (comment) 함께 처리하고 싶지만 #173 이슈가 올라와있어서 우선 merge진행하도록 할게요.
#167 (comment) issue에 추가해놓겠습니다.

@Sunjae95 Sunjae95 mentioned this pull request Jan 3, 2025
@Sunjae95 Sunjae95 merged commit dcbc658 into main Jan 3, 2025
6 checks passed
@Sunjae95 Sunjae95 deleted the 152-sidebar-header-fix branch January 3, 2025 12:16
@yolophg
Copy link

yolophg commented Jan 4, 2025

헤더와 사이드바 사이에 완전히 딱 붙지 않고 공간이 쪼끔 있으면 더 나을 것 같은데 어떻게 생각하세요? @yolophg

동의합니다, 기존에 '풀이현황'과 사이드바/테이블 간의 마진이 존재하고 sticky가 되고 난 이후로도 이 간격이 유지되면 좋을 듯 합니다.
해당 부분 제가 추가로 피그마에도 강조해두고 이번 스프린트에서도 한 번 더 공유드리도록 하겠습니다!
Screenshot 2025-01-04 at 18 46 04

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants