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

[DS-101] Button Svg Size 변경 #144

Merged
merged 4 commits into from
Nov 30, 2023
Merged

[DS-101] Button Svg Size 변경 #144

merged 4 commits into from
Nov 30, 2023

Conversation

LTakhyunKim
Copy link
Contributor

@LTakhyunKim LTakhyunKim commented Nov 29, 2023

DS-101

Button 의 svg 사이즈가 20px 로 잘못 지정된 이슈를 해결합니다.
Toggle Button 도 함께 수정됩니다.

@LTakhyunKim LTakhyunKim added the bug Something isn't working label Nov 29, 2023
@LTakhyunKim LTakhyunKim self-assigned this Nov 29, 2023
Copy link

@deminoth
Copy link
Member

기존 iconStyle 내용을 전부 삭제하고 새로 작성하는게 좋을 것 같습니다. 원래 Mui Icon은 fontSize로 크기를 조절하는데(width, height가 1em이라 fontSize를 따라갑니다) 이런 부분을 존중하지 않고 여러 코드가 섞여있네요.

@LTakhyunKim
Copy link
Contributor Author

LTakhyunKim commented Nov 29, 2023

기존 iconStyle 내용을 전부 삭제하고 새로 작성하는게 좋을 것 같습니다. 원래 Mui Icon은 fontSize로 크기를 조절하는데(width, height가 1em이라 fontSize를 따라갑니다) 이런 부분을 존중하지 않고 여러 코드가 섞여있네요.

코멘트 감사합니다. 👍 기존 mui icon 동작 방식은 놓치고 있었네요.
Lunit Design system 에 따라 font size 를 18px 로 고정 후,
svg width, height 는 기본 동작인 1em 으로 동작하므로,
svg style 코드는 삭제 조치했습니다. 99b67f1

@deminoth
Copy link
Member

수정 후 Button은 문제 없이 나오는데, ToggleButton은 아이콘이 크게 나오는 것 같네요 👀

@LTakhyunKim
Copy link
Contributor Author

LTakhyunKim commented Nov 29, 2023

ToggleButton 은 Button 과 다른 구조로 되어 있어 재수정 했습니다.
구조가 달라 기존에 ToggleButton 에 적용한 iconStyle 은 삭제 했습니다.

향후 유지 보수를 위해 Button, ToggleButton 의 Svg 의 구조를 동일하게 변경했습니다.
(svg 에 Wrapper 컴포넌트가 존재하는 구조)
IconWrapper component 를 추가하고 iconStyle 과 동일한 스타일을 적용합니다. 6f17d36

  1. IconWrapper width, height: 20px
  2. IconWrapper padding: 1px
  3. IconWrapper margin Right: 최대 8px
  4. Icon: 18px 고정 (font size 에 의해 자동 제어)

ToggleButton 컴포넌트에 변경점이 발생, 코드 충돌이 발생하여 해결 후 force push 진행했습니다.
(문제가 있어 추가 수정 진행하겠습니다.) - 수정 - 74b6f17

@LTakhyunKim LTakhyunKim merged commit 96f1bbd into main Nov 30, 2023
5 checks passed
@LTakhyunKim LTakhyunKim deleted the ds-101-button-svg-size branch November 30, 2023 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants