Skip to content

Conversation

@dinohan
Copy link
Member

@dinohan dinohan commented Nov 7, 2022

change the font-weight of bold text from 600 to 'bold'

BREAKING CHANGE: every bold text is changed from 'font-weight:600' to 'font-weight: bold'

Self Checklist

  • I wrote a PR title in English.
  • I added an appropriate label to the PR.
  • I wrote a commit message in English.
  • I wrote a commit message according to the Conventional Commits specification.
  • I added the appropriate changeset for the changes.
  • I wrote a unit test about the implementation.
  • I wrote a storybook document about the implementation.
  • I tested the implementation in various browsers.
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox

Summary

Text 컴포넌트 prop으로 bold={true} 를 받았을 때, font-weight를 600이 아닌 bold로 설정합니다.

Details

Bezier상 Typography의 스펙이 변경되어 대응합니다.

대표 사용처인 채널톡 Desk 어플리케이션과 이 bezier-react storybook에선 다음 폰트 정의을 가져다 쓰고 있습니다.
https://cf.channel.io/asset/font/Inter/inter.css

@font-face {
  font-family: 'Inter';
  font-style:  normal;
  font-weight: 400;
  font-display: swap;
  src: url("Inter-Regular.woff2?v=3.13") format("woff2"),
       url("Inter-Regular.woff?v=3.13") format("woff");
}
@font-face {
  font-family: 'Inter';
  font-style:  italic;
  font-weight: 400;
  font-display: swap;
  src: url("Inter-Italic.woff2?v=3.13") format("woff2"),
       url("Inter-Italic.woff?v=3.13") format("woff");
}

@font-face {
  font-family: 'Inter';
  font-style:  normal;
  font-weight: 700;
  font-display: swap;
  src: url("Inter-Bold.woff2?v=3.13") format("woff2"),
       url("Inter-Bold.woff?v=3.13") format("woff");
}
@font-face {
  font-family: 'Inter';
  font-style:  italic;
  font-weight: 700;
  font-display: swap;
  src: url("Inter-BoldItalic.woff2?v=3.13") format("woff2"),
       url("Inter-BoldItalic.woff?v=3.13") format("woff");
}

이 css의 font-face를 보면 font-weight: 600을 지원하지 않습니다.
그래서 실제로는, 이 storybook이나 데스크 사용처에서 font-weight: 600으로 설정된 텍스트들이 Inter-BoldItalic.woff2 (700에 해당하는 폰트) 로 그려집니다.
(특정 font-weight가 존재하지 않으면 가까운 weight가 사용됩니다.)

When a specified weight doesn't exist, a nearby weight is used.

즉, 기존에 beizer 스펙과 bezier-react Text component에 bold text는 semibold(600) 으로 정의되어 있었지만,
실제로 storybook이나 Desk 서비스에서는 700으로 렌더링 된 폰트를 보고 있었다는 겁니다.

그래서 bezier 스펙에서 bold text의 스펙을 semibold 에서 bold로 바꾸었고,
이에 따라 bezier-react의 Text 컴포넌트를 수정합니다.

사용처에서 변하는 부분

기존에 600으로 표시되던 텍스트들이 bold(700)으로 표시됩니다.
단, 기존 데스크 사용처는 변화가 없습니다.

References

@dinohan dinohan added the fix PR related to bug fix label Nov 7, 2022
@dinohan dinohan self-assigned this Nov 7, 2022
@changeset-bot
Copy link

changeset-bot bot commented Nov 7, 2022

🦋 Changeset detected

Latest commit: 1f0e2d1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@channel.io/bezier-react Minor
bezier-figma-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dinohan dinohan requested a review from sungik-choi November 7, 2022 06:46
@codecov
Copy link

codecov bot commented Nov 7, 2022

Codecov Report

Base: 71.15% // Head: 71.15% // No change to project coverage 👍

Coverage data is based on head (1f0e2d1) compared to base (29c4a62).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff            @@
##           next-v1     #945   +/-   ##
========================================
  Coverage    71.15%   71.15%           
========================================
  Files          208      208           
  Lines         2961     2961           
  Branches       821      821           
========================================
  Hits          2107     2107           
  Misses         736      736           
  Partials       118      118           
Impacted Files Coverage Δ
...es/bezier-react/src/components/Text/Text.styled.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

Chromatic Report

🚀 Congratulations! Your build was successful!

Copy link
Contributor

@sungik-choi sungik-choi left a comment

Choose a reason for hiding this comment

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

오픈소스 라이브러리 관점에서도 boldfont-weight: bold 인게 더 적절한 거 같습니다 👍
이전에 weight 값이 다르다고 이야기했던게 기억나는데, 해결되어서 좋네요

@dinohan
Copy link
Member Author

dinohan commented Nov 7, 2022

Breaking change는 아니어서 첫 번째 커밋 메시지에서 Breaking change를 제거했습니다.
non-breaking change이지만, 사용처에 영향을 주기 때문에 changeset의 minor 버전 표시는 유지합니다.

@dinohan dinohan merged commit 51e4569 into channel-io:next-v1 Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix PR related to bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants