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

refactor: text field props #72

Merged
merged 3 commits into from
Apr 29, 2024
Merged

refactor: text field props #72

merged 3 commits into from
Apr 29, 2024

Conversation

Hanna922
Copy link
Member

@Hanna922 Hanna922 commented Apr 26, 2024

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

4️⃣ 체크리스트 (Checklist)

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

@nijuy
Copy link
Collaborator

nijuy commented Apr 27, 2024

슬랙으로 전달 원하셨는데 같은 얘기를 여러 채널로 주고 받으면 내용이 분산되기도 하고,
시간 지나면 추적이 어려워서 깃허브로 달아두겠습니다

#70 에서 제시하신 4가지 변경사항에 대한 내용을 분리해서 적어보려고 했는데 약간씩 다 연관되어있어 내용이 좀 섞인 거 같네요 양해 부탁드립니다

TextField의 searchPrefix props 제거 ➡ 반대

이전에 TextFieldProps을 유지하되 각 TextField의 props type에서는 사용하지 않는 필드를 Omit으로 제거하는 방법을 고려 중이라고 말씀 드렸는데요. 이 방법으로 기존 SimpleTextField를 수정 / 나머지 TextField를 구현하는 것이 적절할 거 같습니다.

이유는 구현 편의성을 생각했을 때 위 방법이 가장 효율적이라고 생각하기 때문입니다.

TextFieldProps를 수정하면 SearchTextField에서 searchPrefixTextField 사이 위치 조정을 위해 추가적인 스타일 코드가 많이 들어갈 것으로 예상됩니다. (searchPrefix가 input 내에 위치하게 하면서 text와 겹치지 않게 하는 게 까다로울 거 같네요)

하지만 Omit을 사용하면 스타일링 수정 없이도 필요한 필드만 가지게 한다는 장점을 가져갈 수 있습니다.
(같은 이유로 suffix 관련 props도 TextFieldProps 내에 유지하자는 입장입니다.)

SimpleTextField 내부의 공통 TextField 문서 분리

주의사항을 공통 문서로 빼니 SimpleTextField 문서가 훨씬 간결해진 거 같네요! 좋습니다

@Hanna922
Copy link
Member Author

Hanna922 commented Apr 29, 2024

#69 (comment)
해당 코멘트에서 말씀 드렸듯 저는 '3군데에서 모두 사용하지 않는 props는 searchPrefix만 존재'가 고민하던 부분이었습니다.

일단 구현방법이 까다롭다 하시고 YDS 규모 또한 작기 때문에 Omit으로 작업해도 좋을 것 같지만, YDS가 유어슈 내부에서만이 아닌 오픈소스로 나아간다고 생각했을 때, 새로운 TextField, props가 확장될 때마다 Omit으로 하나하나 제거해주는 방법이 좋을지는 같이 생각해보면 좋을 것 같습니다.
특히 searchPrefix와 같이 단 하나의 TextField만을 위해 Base TextField에 props를 추가하는 것은 저로써는 고민이 많이 되는 것 같습니다.

Merge branch 'develop' into feat/#70-TextField-props
@Hanna922 Hanna922 force-pushed the feat/#70-TextField-props branch from 504da86 to c9c3561 Compare April 29, 2024 12:08
@Hanna922 Hanna922 merged commit 34b2d82 into develop Apr 29, 2024
@Hanna922 Hanna922 deleted the feat/#70-TextField-props branch April 29, 2024 13:31
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.

feat: TextField의 suffix 관련 props 제거, docs 수정
2 participants