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

Remove unnecessary size attribute of the TextField's internal input element #1053

Open
sungik-choi opened this issue Dec 3, 2022 · 0 comments
Labels
enhancement Issues or PR related to making existing features better help wanted Issue or PR that extra attention is needed priority:B Issue that important but not urgent

Comments

@sungik-choi
Copy link
Contributor

Description

TextField 내부 input 엘리먼트의 불필요한 size 속성을 제거합니다.

Reasons for suggestion

#1028 에서 TextField 컴포넌트의 사이즈별 폰트 사이즈를 업데이트하며, 내부 input 엘리먼트의 불필요한(하다고 생각한) size 속성을 제거했습니다. TextFieldSize enum은 input 엘리먼트를 감싸는 Wrapper(div) 스타일 컴포넌트에게 주입하여 텍스트 필드의 높이를 결정하는 데 사용하는 값이었기때문에, 잘못 설정된 속성이라고 생각해서 내린 결정이었습니다.

문제는 input 엘리먼트의 size 속성이 유효한 속성이라는 점입니다. 예를 들어 TextFieldSize.M 의 값은 36인데, 이 값을 size 에 주었을 경우 input 엘리먼트는 36개의 글자가 보일 수 있는 만큼의 너비로 설정됩니다. 이 너비는 폰트마다 달라서, Condensed 폰트의 경우와 일반적인 너비를 가진 폰트일 경우의 너비가 다르게 설정됩니다.

이번에 데스크에 bezier-react 마이그레이션을 하면서 어플리케이션 전반적으로 일부 UI에서 input 엘리먼트의 size 속성을 통해 암묵적으로 정해진 너비(M 기준 302px)를 사용하고 있었다는 걸 알게 되었습니다. 이런 케이스가 꽤나 많아서(***Modal, ***Select, 기타 Flex layout), 당장 마이그레이션하기엔 리소스 부족으로 어렵다고 판단했습니다. 이후 Modal, Select 등 컴포넌트를 새로 구현하며 다시 살펴볼 예정입니다.

#1052 를 참고해주세요.

Proposed solution

생략

References

No response

@sungik-choi sungik-choi added help wanted Issue or PR that extra attention is needed component enhancement Issues or PR related to making existing features better labels Dec 3, 2022
@sungik-choi sungik-choi added the priority:A Issue that important and urgent label Jun 21, 2023
@sungik-choi sungik-choi added priority:B Issue that important but not urgent and removed priority:A Issue that important and urgent labels Nov 10, 2023
sungik-choi added a commit that referenced this issue Jan 18, 2024
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] I tested the changes in various browsers. (or didn't have to)
  - Windows: Chrome, Edge, (Optional) Firefox
  - macOS: Chrome, Edge, Safari, (Optional) Firefox

## Related Issue
<!-- Please link to issue if one exists -->

- #1733

## Summary
<!-- Please brief explanation of the changes made -->

Migrate `TextField` component with scss

## Details
<!-- Please elaborate description of the changes -->

- 타이포그래피 믹스인을 추가합니다. TextFieldWrapper 내부에 `Text` 컴포넌트를 추가로 만들 수는 없는
상황인데, 타이포그래피 토큰은 사용해야 했습니다. Text 컴포넌트 내부에서도 해당 믹스인사용하여 중복을 줄이고자 했습니다.
- style, className과 중복되던 inputStyle, inputClassName을 제거. (데스크에서도
className으로 오버라이드해서 사용중)
- TextField named export로 변경
- 내부 useMemo 컴포넌트로 분리 등 리팩토링
- `TextFieldVariant` 의 값을 명시적인 문자열 타입으로 변경
- NOTE) #1053 의 이슈때문에 `TextFieldSize` 의 값은 문자열 타입으로 변경하지 않았습니다.

### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

Yes

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

- testing-library/jest-dom#444 (주석 처리한 테스트 관련.
jest dom이 class 명시도 기반의 스타일링을 제대로 모킹하지 못하는 거 같아요)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues or PR related to making existing features better help wanted Issue or PR that extra attention is needed priority:B Issue that important but not urgent
Projects
Status: 📌 Backlog
Development

No branches or pull requests

1 participant