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

AppBar Component 개발 #24

Merged
merged 42 commits into from
Jul 15, 2024
Merged

AppBar Component 개발 #24

merged 42 commits into from
Jul 15, 2024

Conversation

sean2337
Copy link
Collaborator

@sean2337 sean2337 commented Jul 9, 2024

App Bar Component 개발


🏄🏼‍♂️‍ Summary (요약)

  • App Bar 컴포넌트 개발
  • 뒤로가기 구현

🫨 Describe your Change (변경사항)

  • /src/component/AppBar/AppBar.tsx

🧐 Issue number and link (참고)

📚 Reference (참조)

  • 헤더마다 왼쪽, 오른쪽 컴포넌트가 재각각 달라 AppBar에서 해당 컴포넌트를 제작하는 것보단, 그 페이지마다에서 개발을 진행한 후 그 자체 컴포넌트를 props로 받아서 보여주는게 맞다고 판단했습니다.
    따라서 다음과 같은 Props를 받습니다.
  • Props
  1. title?: string => AppBar에 들어갈 제목
  2. appBarVisible?: boolean => true(default): 헤더가 보여짐, false: 헤더 제거
  3. LeftComp?: React.ReactNode => 왼쪽에 보여질 컴포넌트 default(뒤로가기)
  4. RightComp?: React.ReactNode => 오른쪽에 보여질 컴포넌트

=> 위 내용을 DefaultLayout에 연결해두어 다음과 같이 사용하시면 됩니다.

-사용 예시

<DefaultLayout title="떡잎마을 방법대" RightComp={<>Right Comp</>}>
      ....
</DefaultLayout>
  • 사용 결과
image

@sean2337 sean2337 added 🚀 feature New feature or request 🚧 still working 수정 중입니다. 아직 리뷰하지 말아주세요! labels Jul 9, 2024
@sean2337 sean2337 added this to the v1.0.0 milestone Jul 9, 2024
@sean2337 sean2337 self-assigned this Jul 9, 2024
@klmhyeonwoo
Copy link
Member

klmhyeonwoo commented Jul 9, 2024

늦은 머지 죄송합니다!
혹시 제 커밋 반영해서 앱바 적용까지 같이 넣어주실 수 있으실까요~?

@sean2337 sean2337 removed the 🚧 still working 수정 중입니다. 아직 리뷰하지 말아주세요! label Jul 10, 2024
Copy link
Member

@leeminhee119 leeminhee119 left a comment

Choose a reason for hiding this comment

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

👍 👍 고생하셨습니다!


export function DefaultLayout({ children }: PropsWithChildren) {
type DefaultLayoutProps = PropsWithChildren & AppBarProps;
Copy link
Member

Choose a reason for hiding this comment

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

앗 혹시 PropsWithChildren에 제네릭 타입을 지정하지 않아도 되나요 ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 알기론, PropsWithChildren타입은 props로 children을 받을때 선언해주는걸로 알고 있는데 따로 제네릭 타입을 선정하는 이유가 있을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

결론 부터 말하면 둘 다 똑같은 타입을 반환합니다~!!
다만 예시로 든 type A 같은 경우에는 PropsWithChildren의 정확한 사용 방법은 아니라고 생각합니다.
현재는 제네릭 타입이 단순한 extends라서 별다른 오류가 발생하지 않았지만 다른 타입에선 충분히 이슈가 생길 수 있습니다.
제네릭 타입으로 선언하는 것이 좋아보입니다~!!

type PropsWithChildren<P = unknown> = P & { children?: ReactNode | undefined };

type A = AppBarProps & PropsWidhChildren  // AppBarProps & { children?: ReactNode | undefined };
type B = PropsWithChildren<AppBarProps>  // AppBarProps & { children?: ReactNode | undefined };

Copy link
Member

Choose a reason for hiding this comment

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

옵셔널한 children 외의 props 타입들을 제네릭 타입에 지정해주는 걸로 이해하고 있습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아래와 같이 말씀하시는건가요?? 제가 정확히 이해한지 헷갈려서 여쭈어봐요!!

type DefaultLayoutProps = PropsWithChildren<AppBarProps>;

Copy link
Member

Choose a reason for hiding this comment

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

앗 네네 맞아요 !!

};

//FIXME : 디자인 토큰에 따라 색깔 변경, 폰트 수정
const AppBar = ({ title, appBarVisible = true, LeftComp = <Back />, RightComp = <div></div> }: AppBarProps) => {
Copy link
Member

Choose a reason for hiding this comment

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

잘 몰라서 여쭤봅니다! RightComp에는 기본값이 아무것도 없는 형태라 div 태그를 작성하신 거라면, div 대신 null 또는 빈 태그를 부여하면 어떻게 되나요 ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 의도한 형태는 space between을 적용해서 항상 title이 중앙에 오도록 개발했습니다.
만약 null이나 <></> 이러한 태그를 넣게 될 경우 LeftComp가 왼쪽, Title이 오른쪽으로 가는 의도치 않은 형태가 되어 빈 div태그를 넣어서 RightComp를 넣지 않더라도 Title이 중앙에 오도록 하였습니다.

Copy link
Member

Choose a reason for hiding this comment

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

아하 ~ 그렇군요 이해했습니다!!

@@ -1,18 +1,6 @@
const Login = () => {
export function kakaoLogin() {
Copy link
Member

Choose a reason for hiding this comment

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

어랏 카멜로 작성돼있어요 (파일명도) ! 확인 부탁드려용

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

요거는 페이지는 아니구, 카카오 로그인에 필요한 로직 함수인데 이는 카멜이 맞지 않나요??

Copy link
Member

Choose a reason for hiding this comment

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

아앗 컴포넌트가 아니군요! app 폴더 안에 있어서 컴포넌트라고 제가 착각했네요 하핫 😂

Copy link
Collaborator

Choose a reason for hiding this comment

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

그렇다면 app폴더가 아닌 util 폴더로 빼는 게 좋아보입니다~!!
그리고 로직함수라면 arrow function으로 작성해야할 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네넵 맞습니다. 기존에 만든것 또한 현재 컨벤션이랑 다른게 너무 많아 따로 이슈로 파두고 수정할 예정입니다!!
언급해주셔서 감사합니다 ☺️☺️

Copy link
Member

Choose a reason for hiding this comment

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

저두 너무 많아서 브랜치 새로 파서 수정하려구요.. ㅎㅎ 좋습니다

@sean2337 sean2337 mentioned this pull request Jul 13, 2024
14 tasks
src/layout/DefaultLayout.tsx Outdated Show resolved Hide resolved
@sean2337 sean2337 changed the title feat: #11 Add AppBar Component AppBar Component 개발 Jul 15, 2024
Copy link
Member

@klmhyeonwoo klmhyeonwoo left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ✨

Copy link
Collaborator

@donghunee donghunee left a comment

Choose a reason for hiding this comment

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

수고하셨습니다:)

@sean2337 sean2337 merged commit 75bdc73 into develop Jul 15, 2024
1 check passed
@sean2337 sean2337 deleted the feat/#11/AppBar branch July 15, 2024 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants