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.

Sign up for GitHub

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

Jump to bottom

[Feat] Header GUI 반영, 전체적인 레이아웃 구조 설계 #273

Merged
merged 13 commits into from
Oct 21, 2024
Merged

[Feat] Header GUI 반영, 전체적인 레이아웃 구조 설계 #273

merged 13 commits into from
Oct 21, 2024

Conversation

wuzoo
Copy link
Contributor

@wuzoo wuzoo commented Oct 20, 2024 •
edited
Loading

해당 이슈 번호

closed #266

체크리스트

  • 🔀 PR 제목의 형식을 잘 작성했나요? e.g. [feat] PR을 등록한다.
  • 💯 테스트는 잘 통과했나요?
  • 🏗️ 빌드는 성공했나요?
  • 🧹 불필요한 코드는 제거했나요?
  • ✅ 컨벤션을 지켰나요?
  • 💭 이슈는 등록했나요?
  • 🏷️ 라벨은 등록했나요?
  • 🙇‍♂️ 리뷰어를 지정했나요?
  • ✨ 저는 shared로 분리했어요.

💎 PR Point

기존 레이아웃에서, 아카이빙 페이지만 padding을 주지 않고 다른 모든 페이지에는 공통적으로 존재하는 Header와 padding을 설정해주었습니다. 아카이빙 페이지에서 패딩값을 주게 되면 하나의 요소로 삽입되는 document bar가 정상적으로 작동하지 않기 때문에, 아카이빙 페이지만 제외하였습니다.

Header 컴포넌트와 App에서는 useLocation을 활용하여 pathname이 아카이빙일 시에 헤더를 노출 x / 패딩을 설정 x 하게 됩니다 !

<SNB />
<main css={layoutStyle}>
  <Header />
  <Outlet />
</main>
// ...

현재 위와 같은 구조로 되어있습니다.

layoutStyle 은 아래와 같습니다. flag 값은 아카이빙 페이지일 시에 true가 됩니다 !

const layoutStyle = (flag: boolean) =>
  css({
    display: 'flex',
    flexDirection: 'column',

    height: '100%',
    width: 'calc(100% - 7.6rem)',

    padding: flag ? '0' : '2rem 3.4rem 4.8rem 3.2rem',

    marginLeft: '7.6rem',

    overflow: 'hidden',
  });

document bar의 스타일은 다음과 같습니다 ! 제가 이전에 width를 직접 변경시키게 되면 리플로우가 발생하면서 성능에 안좋은 영향을 끼칠 수 있다고 했는데, 일단 이 방법이 최선이였습니다.. 추후 기회가 된다면 한 번 더 디벨롭해보겠습니다.

📌스크린샷 (선택)

2024-10-20_11.56.59.mov

@wuzoo wuzoo self-assigned this Oct 20, 2024
@github-actions GitHub Actions
Copy link

🚀 Storybook 확인하기 🚀

namdaeun
namdaeun reviewed Oct 20, 2024
View reviewed changes
Copy link
Member

@namdaeun namdaeun left a comment

Choose a reason for hiding this comment

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

블록 이름 빼고 LGTM입니다 ~

src/shared/component/Header/Header.style.ts Outdated
Comment on lines 25 to 29
export const settingIconStyle = css({
'& > path': {
fill: theme.colors.gray_800,
},
});
Copy link
Member

Choose a reason for hiding this comment

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

svg path를 지정해주신 이유가 있을까요 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

기존 피그마에서 헤더에 넘겨준 setting 아이콘에 살짝 디자인과는 조금 다르게 화면에 나오는 이슈가 있어서, 기존에 있던 아이콘을 쓰되, 흰색 path 였기 때문에 바꾸어주었습니당 !

rtttr1
rtttr1 approved these changes Oct 20, 2024
View reviewed changes
Copy link
Contributor

@rtttr1 rtttr1 left a comment

Choose a reason for hiding this comment

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

LGTM~~!!

src/page/archiving/index/component/DocumentBar/DocumentBar.style.ts Outdated
@@ -4,15 +4,14 @@ import { theme } from '@/common/style/theme/theme';

export const containerStyle = (blockSelected: string) =>
css({
position: 'sticky',
position: 'fixed',
Copy link
Contributor

Choose a reason for hiding this comment

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

오 sticky 변경했네요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이친구는 일단 디자인의 요청사항대로 왼쪽 컨텐츠들이 밀리면서 사이드바가 나오는 형태의 구현을 실패하여서.. 임시로 테스트해보다가 바꾸고 커밋한거 같습니다.

디자이너분들과 얘기 끝나면 따로 이슈파서 진행하면 될 것 같네요 !

cindy-chaewon
cindy-chaewon approved these changes Oct 20, 2024
View reviewed changes
Copy link
Contributor

@cindy-chaewon cindy-chaewon left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!! LGTM~~~

@github-actions GitHub Actions
Copy link

🚀 Storybook 확인하기 🚀

Bowoon1216
Bowoon1216 reviewed Oct 20, 2024
View reviewed changes
Copy link
Contributor

@Bowoon1216 Bowoon1216 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 ! 덕분에 멋진 헤더 ~~ 👍

src/App.tsx Outdated

/** 아카이빙 페이지 DocumentBar를 위한 라우트별 동적 패딩 */
const isArchivingPage = pathname === '/archiving';
console.log(pathname);
Copy link
Contributor

Choose a reason for hiding this comment

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

console.log 찾아냈습니다 ✌️

@wuzoo wuzoo changed the title [Fea] Header GUI 반영, 전체적인 레이아웃 구조 설계 [Feat] Header GUI 반영, 전체적인 레이아웃 구조 설계 Oct 20, 2024
@github-actions GitHub Actions
Copy link

🚀 Storybook 확인하기 🚀

@github-actions GitHub Actions
Copy link

🚀 Storybook 확인하기 🚀

@wuzoo wuzoo merged commit 23e9454 into develop Oct 21, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bowoon1216 Bowoon1216 Bowoon1216 left review comments

@namdaeun namdaeun namdaeun left review comments

@cindy-chaewon cindy-chaewon cindy-chaewon approved these changes

@rtttr1 rtttr1 rtttr1 approved these changes

Assignees

@wuzoo wuzoo

Labels
⭐️주용 💄Design
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Header 컴포넌트 GUI 반영 및 레이아웃 설계
5 participants
@wuzoo @rtttr1 @namdaeun @cindy-chaewon @Bowoon1216