Feat: 공통 Header를 구현합니다. #19

jungwoo3490 · 2025-01-28T13:49:47Z

close #7

☑️ 완료 태스크

공통 Header 구현

🔎 PR 내용

Interface 설계

와이어프레임을 확인해보면 Header의 중앙 Element는 항상 존재하기 때문에 required prop으로 지정했고, left, right Element는 존재하지 않을 가능성도 있기 때문에 optional prop으로 지정했어요.

Element 배치 설계

처음에는 좌우 padding을 주고 display: flex, justify-content: space-between으로 배치하려고 했으나, left, right Element가 optional prop이므로, 존재하지 않게 되면 위치가 다 깨져버리는 문제가 있었어요. 그래서 optional prop이 존재하지 않는 경우도 올바른 위치를 보장하기 위해 각각 Header 내에서 절대적인 위치를 잡아주었어요. 중앙 Element는 margin: 0 auto로 중앙정렬, 좌측, 우측 Element는 position: absolute로 각각 Header 내부 좌측 우측에 고정시켜주었어요. 그 결과 optional prop이 존재하지 않는 경우도 올바른 위치를 보장하게 됩니다!!

좌측 Element X

우측 Element X

좌, 우측 Element X

웹 접근성 향상을 위한 aria-label

웹 접근성 향상을 위해 다음과 같이 "페이지 헤더"라는 aria-label을 추가했어요.

Header 상단 고정

Header를 상단고정하기 위해 처음에는 position: fixed를 사용했어요. 그러다보니 문서 흐름을 벗어나버려서 page body 부분이 Header 밑으로 깔리게 되는 문제가 있었어요.

이를 해결하기 위해 페이지 공통 레이아웃의 Outlet 부분이 Header의 높이만큼 상단 margin을 갖게 하여 body 부분이 가려지지 않도록 해주었어요. 해결은 되었지만 좋은 방법이라는 생각이 들지 않았어요. 만약 Header의 height이 변경되면 그때마다 Outlet 영역의 상단 margin도 항상 sync를 맞춰주어야 하기 때문에 유지보수 측면에서 좋지 못하다고 생각했기 때문이에요. 더 나은 방법을 고민하던 중에 서현이가 알려준 대로 position: sticky를 사용하니 별도로 상단 margin을 주지 않고도 해결되었어요.

position: sticky는 초기에 relative처럼 동작해요. 그러다가 부모 스크롤 영역 안에서 지정해준 position에 도달하면 fixed처럼 고정되는데, 이때 position: fixed처럼 문서 흐름에서 벗어나서 독립적인 레이어에서 동작하는게 아니라, 문서 흐름을 유지하면서 지정한 위치에 고정돼요. 그래서 고정이 되면서도 page body를 가리지 않게 되는 원리입니다!!

🙋🏻‍♂️ 논의가 필요한 점

HeaderWrapper가 flex 속성까지 담당하는 방법과 HeaderWrapper 자식 컴포넌트로 Flex 컴포넌트를 위치시켜 Flex 컴포넌트에게 flex에 관련된 속성을 위임하는 방법 중 고민했는데, depth 증가 + align="center"를 위해 Flex 컴포넌트가 HeaderWrapper의 높이를 상속받아야 하는 것이 불필요하다고 생각해서 HeaderWrapper에서 flex 속성까지 담당하는 방법을 선택했어요. 연서 PR에서도 논의되었던 부분과 같은 결의 내용 같은데, 다음 회의때 결정될 것 같은 부분이라 임시로 이 방법을 택했다고 봐주시면 감사하겠습니다🙏🏻
prop 네이밍은 다양한 프로젝트의 header 인터페이스를 참고해서 가장 general하게 사용되는 이름으로 네이밍해보았는데, 너무 길다는 생각이 든다면 언제든 더 나은 네이밍을 추천해주셔도 좋습니다!!

📷 스크린샷

…erty

…eat/#7-header

seobbang

prop 네이밍은 다양한 프로젝트의 header 인터페이스를 참고해서 가장 general하게 사용되는 이름으로 네이밍해보았는데, 너무 길다는 생각이 든다면 언제든 더 나은 네이밍을 추천해주셔도 좋습니다!!

저는 직관적이고 좋은 것 같습니다!

고생 많으셨어용 ~~ ✨👍

seobbang · 2025-02-02T08:52:21Z

src/common/components/Header.tsx

+const LeftElement = styled.div`
+  position: absolute;
+  left: 1.5rem;
+`;
+
+const CenterElement = styled.div`
+  margin: 0 auto;
+`;
+
+const RightElement = styled.div`
+  position: absolute;
+  right: 1.5rem;
+`;


Suggested change

const LeftElement = styled.div`

position: absolute;

left: 1.5rem;

`;

const CenterElement = styled.div`

margin: 0 auto;

`;

const RightElement = styled.div`

position: absolute;

right: 1.5rem;

`;

const LeftElement = styled.div({

position: absolute;

left: 1.5rem;

});

회의 때 얘기 나눈 것처럼 string tagged template 말고 object 사용하면 좋을 것 같아요~!

수정해두었습니다!! (626a4dd)

seobbang · 2025-02-02T09:19:36Z

src/common/components/Header.tsx

+
+const HeaderWrapper = styled.header({
+  position: "sticky",
+  zIndex: 999,


요런 공통 컴포넌트에서 쓰이는 z-index 값들은 다른 요소들의 z-index 와 영향을 미칠 수 있으니까
export const HEADER_Z_INDEX = 999
이런식으로 하나 상수 파일 안에서 z-index들 관리해도 좋을 것 같아요!

그래서 헤더보다 낮은 z index를 줘야할 때 예를 들면
zIndex : HEADER_Z_INDEX - 1 이런식으로 관리하면 유용하더라구용

zIndex를 상수로 관리하는 의견에 동의해요!
다른 곳에서도 zIndex를 사용하는 곳이 있을수 있어서, 아예 글로벌로 스케일을 정해두고 사용해도 좋을것 같습니다!

// ex. common style 파일에 다음과 같이 zIndex 스케일을 정해두고, 사용시에는 z-index: ${zIndicies.global1};의 형태로 사용가능해요 ! export const zIndicies = { global1: 900, global2: 800, page1: 80, page2: 70, page3: 60, component1: 30, component2: 20, component3: 10, negative: -10, };

상수로 관리하는 거 좋은 것 같아요!! 좋은 의견 감사해요🙏🏻
글로벌 스케일 방식 괜찮아보이는데 저희가 디자인 시스템이 없다 보니 따로 elevating guide 같은게 없어서 글로벌 스케일을 정확히 어떤 수치로 나누어 정의해야할지 조금은 모호한 부분이 있네요...

// src/constants/zIndex.ts export const Z_INDEX = { MODAL: 1000, DROPDOWN: 100, HEADER: 10, } as const;

임의로 이렇게 구현해서 사용하는 방식으로 변경해보았어요. (0252fdc)
다음 프론트 회의 때 글로벌 스케일에 대한 부분도 같이 논의해보고 싶어요!! 🙌🏻

seobbang · 2025-02-02T09:39:51Z

src/common/components/Header.tsx

+const LeftElement = styled.div`
+  position: absolute;
+  left: 1.5rem;
+`;


여기랑 RightElement에 height: "100%" 주는거 어떨까요?
여기서 줘야 leftElement 전달할 때 height 100% 사용이 가능해서요!

@seobbang

여기에 height: 100%를 지정하지 않은 이유는 HeaderWrapper에서 각 Element를 flex align center로 중앙정렬을 해뒀기 때문이에요!!

height: 100%를 지정하게 되면 결국 자동 중앙정렬의 효과가 사라질테고, 그렇다고 각 Element에서 중앙정렬을 각각 알아서 주어야 하는 것은 반복적이고 비효율적인 작업이라고 생각했어요!! 또 Header가 현재 항상 고정된 높이를 가지기 때문에 저희가 자식 Element의 높이로 % 단위를 사용할 일이 있을까 싶기도 하구요!!

어떻게 생각하시나요?? 👀

seobbang · 2025-02-02T09:49:25Z

src/common/components/Header.tsx

+  return (
+    <HeaderWrapper aria-label="페이지 헤더">
+      <LeftElement>{leftElement}</LeftElement>
+      <CenterElement>{centerElement}</CenterElement>


Suggested change

<CenterElement>{centerElement}</CenterElement>

<CenterElement as={typeof centerElement === 'string' ? 'h1' : 'div'}>{centerElement}</CenterElement>

이렇게 지정해주면 어떨까용?

오홍 emotion/styled 에서 as prop 지원하는거 첨 알았네요...! 이거 넘 좋네요 🚀🚀
3f2f03e에서 반영 완료했습니다~!

Yeonseo-Jo

수고하셨습니댜 ~~ 덕분에 저도 sticky에 대해 다시 배워가요

prop 네이밍 깔끔하고 좋아요 ~! 이대로 가시죠
추가적인 의견 하나 남기자면! 명확히 디자인이 있는것은 아니지만 헤더랑 페이지 배경 색이 같은 경우 이미지처럼 헤더 하단 border가 있으면 더 깔끔해보이는것 같아서 border 옵션 prop 넣어주거나 적용시켜주면 좋을것 같습니당👍

Yeonseo-Jo · 2025-02-02T13:44:47Z

src/common/components/Header.tsx

+
+const HeaderWrapper = styled.header({
+  position: "sticky",
+  zIndex: 999,


zIndex를 상수로 관리하는 의견에 동의해요!
다른 곳에서도 zIndex를 사용하는 곳이 있을수 있어서, 아예 글로벌로 스케일을 정해두고 사용해도 좋을것 같습니다!

// ex. common style 파일에 다음과 같이 zIndex 스케일을 정해두고, 사용시에는 z-index: ${zIndicies.global1};의 형태로 사용가능해요 ! export const zIndicies = { global1: 900, global2: 800, page1: 80, page2: 70, page3: 60, component1: 30, component2: 20, component3: 10, negative: -10, };

Yeonseo-Jo · 2025-02-02T14:05:57Z

src/common/components/Header.tsx

+  leftElement?: ReactNode;
+  centerElement: ReactNode;
+  rightElement?: ReactNode;


공통컴포넌트인만큼 타입 범위를 보다 명확히 좁혀줘도 좋을것 같아요!

Suggested change

leftElement?: ReactNode;

centerElement: ReactNode;

rightElement?: ReactNode;

leftElement?: ReactElement | string ;

centerElement: ReactElement | string;

rightElement?: ReactElement | string;

두 타입 이외에 다른 타입 들어올 일 없을 것 같아서 훨 좋아보이네요!!!
9efe246에서 반영해두었습니당 😃

Brokyeom · 2025-02-04T19:28:25Z

안녕하세요, 익숙한 이름들이 보이는 레포라 슬쩍 와서 코멘트 남기고 갑니다..

prop명으로 사용되는 *Element 라는 이름도 ReactElement 라는 타입을 받고있기 때문에, 크게 어색하지 않지만, string 타입이 함께 들어오는 옵션이다보니, 좀 더 포괄적인 네이밍인 *Addon 은 어떤가 제안드려요.
공통으로 사용되는 컴포넌트의 경우에, 복잡한 스타일이 존재하는 경우가 아니라면, emotion의 css prop으로 빌드타임 정적 css로 스타일을 미리 생성해 성능상 이득을 챙겨보는건 어떨까 싶어요. 요놈이 emotion을 zero-runtime 처럼 쓰게 해주는 유용한 녀석이랍니다.

새벽에 과제하다가 머리 안돌아가서 기웃거려 봅니다 그럼 파이팅~~

jungwoo3490 · 2025-02-05T14:50:50Z

@Brokyeom 좋은 리뷰 감사합니다 🙌🏻🙌🏻

prop명으로 사용되는 *Element 라는 이름도 ReactElement 라는 타입을 받고있기 때문에, 크게 어색하지 않지만, string 타입이 함께 들어오는 옵션이다보니, 좀 더 포괄적인 네이밍인 *Addon 은 어떤가 제안드려요.

Addon이라는 이름도 좋아보여요!! 그런데 Addon이라는 네이밍 특성상 optional한 감이 좀 있어서, required prop인 CenterElement에 적용하기에는 조금 어색하다고 느껴져요. 보통 필수 Element는 어떻게 네이밍을 하는 편인지 궁금해요!!

emotion의 css prop으로 빌드타임 정적 css로 스타일을 미리 생성해 성능상 이득을 챙겨보는건 어떨까 싶어요. 요놈이 emotion을 zero-runtime 처럼 쓰게 해주는 유용한 녀석이랍니다.

저는 emotion의 css prop이 runtime에 스타일이 적용되는 것으로 이해하고 있어요!! 빌드타임에 @emotion/babel-plugin이 아래와 같이 css prop을 사용한 JSX를 jsx 함수 호출로 변환하고 className prop으로 변경하는 단계까지만 진행해주고,

컴파일 전

<div
  css={{
    color: "red",
  }}
>
  안녕
</div>

컴파일 후

jsx('div', {
 className: css({
   color: 'red'
 })
}, '안녕')

실제 css 함수가 실행되어 스타일이 주입되는 시점은 runtime이라고 알고 있어요!!

jungwoo3490 added 4 commits January 28, 2025 22:18

feat: add Header component

c158382

chore: add preview code

1c64a58

fix: remove Flex component and change HeaderWrapper to have flex prop…

f663a4a

…erty

Merge branch 'main' of https://github.com/Mo-yoy/Moyo-Frontend into f…

a597220

…eat/#7-header

jungwoo3490 added ✨ Feature 기능 개발 🐻 쩡우 labels Jan 28, 2025

jungwoo3490 requested review from Yeonseo-Jo and seobbang January 28, 2025 13:49

jungwoo3490 self-assigned this Jan 28, 2025

pull-request-size bot added the size/M label Jan 28, 2025

seobbang reviewed Feb 2, 2025

View reviewed changes

Yeonseo-Jo reviewed Feb 2, 2025

View reviewed changes

jungwoo3490 added 4 commits February 4, 2025 01:35

feat: dynamically change CenterElement tag based on content type

3f2f03e

fix: narrow HeaderProps types to ReactElement or string

9efe246

fix: convert styled components to object syntax

626a4dd

fix: manage z-index values using constants

0252fdc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: 공통 Header를 구현합니다. #19

Feat: 공통 Header를 구현합니다. #19

jungwoo3490 commented Jan 28, 2025 •

edited

Loading

seobbang left a comment

seobbang Feb 2, 2025

jungwoo3490 Feb 3, 2025

seobbang Feb 2, 2025

Yeonseo-Jo Feb 2, 2025

jungwoo3490 Feb 3, 2025

seobbang Feb 2, 2025 •

edited

Loading

jungwoo3490 Feb 4, 2025 •

edited

Loading

seobbang Feb 2, 2025

jungwoo3490 Feb 3, 2025

Yeonseo-Jo left a comment

Yeonseo-Jo Feb 2, 2025

Yeonseo-Jo Feb 2, 2025

jungwoo3490 Feb 3, 2025 •

edited

Loading

Brokyeom commented Feb 4, 2025 •

edited

Loading

jungwoo3490 commented Feb 5, 2025 •

edited

Loading

	<CenterElement>{centerElement}</CenterElement>
	<CenterElement as={typeof centerElement === 'string' ? 'h1' : 'div'}>{centerElement}</CenterElement>

Feat: 공통 Header를 구현합니다. #19

Are you sure you want to change the base?

Feat: 공통 Header를 구현합니다. #19

Conversation

jungwoo3490 commented Jan 28, 2025 • edited Loading

☑️ 완료 태스크

🔎 PR 내용

Interface 설계

Element 배치 설계

좌측 Element X

우측 Element X

좌, 우측 Element X

웹 접근성 향상을 위한 aria-label

Header 상단 고정

🙋🏻‍♂️ 논의가 필요한 점

📷 스크린샷

seobbang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seobbang Feb 2, 2025 • edited Loading

Choose a reason for hiding this comment

jungwoo3490 Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yeonseo-Jo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungwoo3490 Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Brokyeom commented Feb 4, 2025 • edited Loading

jungwoo3490 commented Feb 5, 2025 • edited Loading

컴파일 전

컴파일 후

jungwoo3490 commented Jan 28, 2025 •

edited

Loading

seobbang Feb 2, 2025 •

edited

Loading

jungwoo3490 Feb 4, 2025 •

edited

Loading

jungwoo3490 Feb 3, 2025 •

edited

Loading

Brokyeom commented Feb 4, 2025 •

edited

Loading

jungwoo3490 commented Feb 5, 2025 •

edited

Loading