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

[SP2] 드롭다운 컴포넌트 추가, 프로젝트 페이지 필터에 적용 #223

Merged
merged 8 commits into from
Oct 23, 2023

Conversation

SeojinSeojin
Copy link
Member

@SeojinSeojin SeojinSeojin commented Oct 19, 2023

Summary

  • 드롭다운 컴포넌트를 도입하여 프로젝트 페이지의 필터에 적용합니다.
  • 드롭다운 컴포넌트 스펙은 플그와 똑같이 했지만 내부 코드는 다릅니다 ..!!

Screenshot

2023-10-19.7.54.37.mov

Comment

  • 일단 드롭다운 코드가 먼저 머지되어야 해서 프로젝트탭 다른 스타일은 손보지 않았어요. 후속 PR에서 다룰 예정입니다! 따라서 단독으로 배포되면 안 되고, 그 PR과 함께 배포되어야 해요.
  • 그래서 이 PR에서는 첫번째 커밋과 두번째 커밋만 보셔도 괜찮고, 다른 커밋에서는 Select 컴포넌트 불러서 쓰는 부분만 봐주셔도 괜찮아요! 프로젝트페이지 전체와 관련한 리뷰는 다음 PR에서 부탁드리겠습니다!!!
  • Select 컴포넌트는 스펙을 표현하기 위한 최소한의 prop을 들고 있게 해두었어요. 확장하기 편한 구조 말씀주신다면 최대한 반영하겠습니다!
  • 스펙 상 드롭다운 바깥을 누르면 닫혀야 해서 useOutsideClickListener이라는 훅을 추가하였어요

@SeojinSeojin
Copy link
Member Author

CI가 돌지 않은 이유는 .. 제가 레거시 필터코드를 제거하지 않았기 때문입니다 ㅜㅜ


return (
<div>
<St.SelectTrigger
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion;

저희 컨벤션에 style 을 S.${스타일네임}로 하기로 했어요!
image

Copy link
Member Author

Choose a reason for hiding this comment

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

S였군요 .. 후딱 고치겠습니다!! 정말 고마워요!!!!

@@ -34,3 +34,5 @@ type TabTypeOption<T> = {

export type TabType = TabTypeOption<Part>;
export type ExtraTabType = TabTypeOption<ExtraPart>;

export type LabelKeyType = string | number | symbol;
Copy link
Contributor

Choose a reason for hiding this comment

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

question;

key 값의 타입으로 symbol이 오는 경우는 어떤게 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

Record에서 key 자리에 올 수 있는 타입이 string, number, symbol이라서 일단 이렇게 적어두었어요 ..!
저희는 enum값으로 string이나 number만 쓰게 될 것 같긴 합니다!

Comment on lines +33 to +34
const selectItemWrapperRef = useRef<HTMLDivElement>(null);
const selectTriggerRef = useRef<HTMLButtonElement>(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

question;

Item과 Trigger 을 각각 div, button 분리해 다른 ref를 사용한 이유가 있나요?
둘 다 같은 ref를 참조해도 되는거 아닌가 싶어서요!

Copy link
Member Author

Choose a reason for hiding this comment

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

같은 ref를 참조하면 더 밑에 있는 ref가 위에 있는 ref를 덮어쓰게 되지 않나요 ..??!
같은 ref를 참조하는 상황에 대하여 더 자세히 설명 주시면 감사하겠습니닷..!

@SeojinSeojin
Copy link
Member Author

  • 리뷰 반영하고, (St->S)
  • 사용되지 않는 컴포넌트 코드 일단 제거해서

올릴게요!!!

@SeojinSeojin SeojinSeojin merged commit 00242c7 into develop Oct 23, 2023
1 check passed
@SeojinSeojin SeojinSeojin deleted the feat/#219_dropdown branch October 23, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

드롭다운 만들기
2 participants