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: <Picker> 구현 #14

Merged
merged 8 commits into from
Nov 21, 2023
Merged

feat: <Picker> 구현 #14

merged 8 commits into from
Nov 21, 2023

Conversation

HyunsDev
Copy link
Contributor

@HyunsDev HyunsDev commented Oct 17, 2023

1️⃣ 어떤 작업을 했나요? (Summary)

기존 코드에 영향을 미치지 않는 변경사항

  • useDOMRef 훅 추가

  • useUncontrolledValue() 훅 추가

  • <Picker> 컴포넌트 추가

2️⃣ 알아두시면 좋아요!

<Picker> 컴포넌트의 구현이 조금 복잡하게 되어 있어요. 주석의 개선이나 코드를 조금 더 깔끔하게 할 수 있는 방법이 있다면 알려주세요!

3️⃣ 추후 작업

4️⃣ 체크리스트 (Checklist)

  • main 브랜치의 최신 코드를 pull 받았나요?

@HyunsDev HyunsDev changed the title Feat/picker feat: <Picker> 구현 Oct 17, 2023
@HyunsDev HyunsDev added the feat label Oct 17, 2023
Copy link
Collaborator

@nijuy nijuy left a comment

Choose a reason for hiding this comment

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

👍 고생하셨습니다!! 리뷰가 쪼끔 늦었죠 호호... 코드도 읽어보고 스토리북으로 이것저것 해보는데 다 잘돼서 넘우 신기했습니다 이런 걸 다 고려하다니 증말 세심하군요 현스...... 궁금한 점이랑 수정 필요한 부분 코멘트 달아놨어요!!!

src/components/Picker/Picker.style.ts Outdated Show resolved Hide resolved
src/components/Picker/Picker.tsx Outdated Show resolved Hide resolved
src/components/Picker/PickerColumn.tsx Show resolved Hide resolved
src/components/Picker/PickerColumn.tsx Outdated Show resolved Hide resolved
import { PickerProps } from './Picker.type';
import { StyledPicker } from './Picker.style';

export const Picker = forwardRef<HTMLDivElement, PickerProps>(({ children, ...props }, ref) => {
Copy link
Member

Choose a reason for hiding this comment

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

Picker 한 개당 최대 3개의 PickerColumn을 가질 수 있다는 제한 사항이 있어서 갯수 제한을 설정해주시면 더 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 디자인적으로만 고려해도 충분할 것 같고, 추후 확장 가능성을 위해 의도적으로 추가하지 않았어요.

갯수 제한을 걸려면 context로 컴포넌트의 수를 센 다음에 3개 초과면 오류를 던저야 하는데, 이 정도의 오버헤드를 감수하면서 개발 단에서 제한을 추가할 필요는 없을 것 같습니다.

src/components/Picker/PickerColumn.tsx Show resolved Hide resolved
src/components/index.ts Show resolved Hide resolved
src/components/Picker/Picker.tsx Show resolved Hide resolved
@nijuy nijuy mentioned this pull request Oct 31, 2023
1 task
@nijuy nijuy removed the request for review from yoo-jimin127 November 21, 2023 13:48
@Hanna922 Hanna922 merged commit 92c8d06 into develop Nov 21, 2023
@Hanna922 Hanna922 deleted the feat/picker branch November 21, 2023 13:49
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.

3 participants