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: TopBar 구현 #74

Merged
merged 6 commits into from
Apr 15, 2024
Merged

feat: TopBar 구현 #74

merged 6 commits into from
Apr 15, 2024

Conversation

hae-on
Copy link
Contributor

@hae-on hae-on commented Apr 5, 2024

Issue

✨ 구현한 기능

피그마에 있는 TopBar를 구현하였습니다.

사실 아직 완성 덜 했는데 ㅎ...
여러분의 의견이 필요해서 올립니다,,,
논의하고 싶은 내용에 적어둘게요!

compound component pattern으로 구현했습니다.
맞게 구현 했는지 봐주세요!

Search2와 ArrowLeft 아이콘 교체했습니다.
이번주 피그마보면
스크린샷 2024-04-06 오전 1 15 28
이런식으로 적혀있는데, 변경된 부분을 표시했다고 합니다.
일단 여기서는 저 두 아이콘을 사용해서 바꿨는데, 사이즈가 저번 아이콘과 달라서 저 둘을 사용하는 곳이 이상하게 보일 수도 있어요.
그때는 상황에 맞게 사이즈 조절해서 쓰면 됩니다~~

검색 부분의 경우 아직 타미의 검색이 머지가 안돼서 그냥 뒀습니다!
검색 머지 되면 병합 후 진행하도록 하겠습니다.

TopBar 컴포넌트 중에 LeftNavigationGroup 이 친구가 있는데 왜 얘만 같이 묶었나면
지금 container에 space-between이 있거든요?
left 일 때는 얘가 적용되면 안되는데, 그러면 props를 또 받아서 그때는 space-between이 적용 안되게 하는 방법밖에 생각이 안나서...
그건 너무 더러워서 그냥 묶어버렸습니다.
혹시 좋은 해결 방안이 있다면,,,
일단 디자인 시안에서는 Title이 왼쪽에 있으면 무조건 뒤로가기 버튼과 함께 있습니다!

아 그리고 props를 하나에 다 때려넣었는데요.
각 컴포넌트별로 또 따로 만들까 했는데 (ex. RegisterLinkProps 등등...)
그러면 Props로 너무 길이 차지할 거 같아서 그냥 한 곳에 넣었습니다.
그래서 어쩔 수 없이 다 옵셔널 값으로 들어갔네요...

옵셔널 값의 문제로 RegisterLink 컴포넌트를 보면 link를 받거든요?
근데 Link to에는 undefined가 들어갈 수 없잖아여.
그래서 계속 오류나서 기본값 그냥 빈값으로 넣었는데 이거 기본값 뭐 줘야하죠?
저 컴포넌트 자체에서는 link가 필수로 들어올건데 옵셔널 때문에 ㅡㅡ 아오

📢 논의하고 싶은 내용

  1. 디자이너분들이 TopBar에 넣어둔 거 보면, 아래 사진처럼 로고도 포함하고 있는데, 이것도 포함할까요??
스크린샷 2024-04-06 오전 1 19 58
  1. 여기에 title이 h1 태그로 쓰이고 있는데, text 컴포넌트는 p랑 span만 받잖아여. 그냥 두고 나중에 heading 컴포넌트를 따로 만들까요?? 일단은 그냥 색상 css로 때려넣었습니다.

  2. 위랑 비슷한데, text 컴포넌트를 쓰려고 했는데, 거기에 없는 색상이면 어떻게 하나요? RegisterLink를 보면, 원래 color가 disabled가 아니거든요? 없는 색상 입력을 못하니까 그냥 비슷한 색으로 제가 넣어버렸습니다...ㅎ....

  3. ⚠️중요!!!!⚠️ 사실 이거 물어보려고 PR 올린건데, 스토리북처럼 쓰실래여 (TopBar.stories.tsx 참고) 아니면 따로 컴포넌트를 만들까요?? 스토리북처럼 쓰면 저 코드 사용처에서 경우에 맞게 하나씩 치면 되구여. 따로 컴포넌트 만드는거면 이제 저걸 묶어서 컴포넌트 이름으로 export 하는거죠. 만약 컴포넌트를 만든다고 하면, 하나의 컴포넌트에서 여러개를 export 하지 않을까 생각합니다! 저걸 또 다 다른 컴포넌트로 하면, 너무 많아질 거 같아서...! 아니면 더 좋은 방법이 있다면 의견 plz,,,🙇‍♂️

🎸 기타

x

⏰ 일정

  • 추정 시간 : 2시간
  • 걸린 시간 : 3시간

@hae-on hae-on requested review from xodms0309 and Leejin-Yang April 5, 2024 16:32
@hae-on hae-on self-assigned this Apr 5, 2024
Copy link

github-actions bot commented Apr 5, 2024

🔗 배포된 storybook 주소: https://65f015a4aed45406385006ee-pnrcasfpwa.chromatic.com/

Copy link
Member

@xodms0309 xodms0309 left a comment

Choose a reason for hiding this comment

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

디자이너분들이 TopBar에 넣어둔 거 보면, 아래 사진처럼 로고도 포함하고 있는데, 이것도 포함할까요??

좋아요

여기에 title이 h1 태그로 쓰이고 있는데, text 컴포넌트는 p랑 span만 받잖아여. 그냥 두고 나중에 heading 컴포넌트를 따로 만들까요?? 일단은 그냥 색상 css로 때려넣었습니다.

넹넹 안그래도 Heading 컴포넌트 하나 있으면 좋을 것 같다는 생각했어요 굿

위랑 비슷한데, text 컴포넌트를 쓰려고 했는데, 거기에 없는 색상이면 어떻게 하나요? RegisterLink를 보면, 원래 color가 disabled가 아니거든요? 없는 색상 입력을 못하니까 그냥 비슷한 색으로 제가 넣어버렸습니다...ㅎ....

이거 디자이너 분한테 통일해달라고 하실래요? 왜냐면 이게 색상을 너무 자유롭게 받아버리면 디자인시스템의 의미가 없다고 생각해서..

⚠️중요!!!!⚠️ 사실 이거 물어보려고 PR 올린건데, 스토리북처럼 쓰실래여 (TopBar.stories.tsx 참고) 아니면 따로 컴포넌트를 만들까요?? 스토리북처럼 쓰면 저 코드 사용처에서 경우에 맞게 하나씩 치면 되구여. 따로 컴포넌트 만드는거면 이제 저걸 묶어서 컴포넌트 이름으로 export 하는거죠. 만약 컴포넌트를 만든다고 하면, 하나의 컴포넌트에서 여러개를 export 하지 않을까 생각합니다! 저걸 또 다 다른 컴포넌트로 하면, 너무 많아질 거 같아서...! 아니면 더 좋은 방법이 있다면 의견 plz,,,🙇‍♂️

그 밖에서 쓸 때 말하시는건가용? 스토리북처럼 쓰는거면

  <TopBar>
    <TopBar.BackLink />
    <TopBar.Title title="타이틀" />
    <TopBar.Spacer />
  </TopBar>

이거고 컴포넌트로 쓰는거면

<CenterTitleAndSearch />

이런식으로 되는걸까요???

코드는 너무 깔끔해요 👍

@hae-on
Copy link
Contributor Author

hae-on commented Apr 6, 2024

@xodms0309

얍얍 저도 통일하는게 좋을 거 같아요! 안그러면 어딘가 또 빵꾸나는 부분이 생길 거 같음!

넹 타미가 말한 그대로입니다! 두 방법중에 어떤게 더 맘에 드시죵???

Copy link
Contributor

@Leejin-Yang Leejin-Yang left a comment

Choose a reason for hiding this comment

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

너무 좋은데요 해온??
한결 깔끔해져서 좋습니다.!

로고 넣어주시면 정말 감사하고 그리고 헤딩 컴포넌트 찬성
각각 컴포넌트를 따로 안 만들어도 될거 같고 지금처럼 사용하면 좋아보입니다

수고했어요 👍

@xodms0309
Copy link
Member

넹넹 저도 지금 방법 좋아요
요 PR에서 나머지도 다 작업하시나요?

@hae-on
Copy link
Contributor Author

hae-on commented Apr 7, 2024

@xodms0309

어떤 나머지 작업 말씀하시는걸까용??
지금 남은 작업은

  1. 로고 (다음주 회의까지 만드신다고 함) 추가
  2. 검색 (이건 타미 태그 검색 머지되면 가져올 것)

요정도입니다!

@xodms0309
Copy link
Member

넹넹 그거 물어보는거 맞아여! 굿굿

Copy link

🔗 배포된 storybook 주소: https://65f015a4aed45406385006ee-eroquxwyar.chromatic.com/

@hae-on
Copy link
Contributor Author

hae-on commented Apr 13, 2024

@xodms0309

검색도 추가하려고 했는데, props로 받는게 많아서 따로 추가 안했습니다!
그냥 사용처에서 바로 SearchInput 넣어서 쓰면 될 거 같아요.
이거 머지하구 새로 이슈 판 다음에 작업할게요~~

@hae-on hae-on merged commit e220fe2 into feat/v2 Apr 15, 2024
2 of 3 checks passed
@hae-on hae-on deleted the feat/issue-73 branch April 15, 2024 07:52
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.

topBar 구현
3 participants