Set up SCSS module environment(ts, storybook) and change `Spinner` to classname-based styling #1488

sungik-choi · 2023-07-11T13:12:26Z

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.
[Component] I wrote a unit test about the implementation.
[Component] I wrote a storybook document about the implementation.
[Component] I tested the implementation in various browsers.
- Windows: Chrome, Edge, (Optional) Firefox
- macOS: Chrome, Edge, Safari, (Optional) Firefox
[New Component] I added my username to the correct directory in the CODEOWNERS file.

Related Issue

Remove the CSS-in-JS library #1106

Summary

Spinner 에 class name 기반의 스타일링을 적용합니다.
SCSS 기반 개발 환경을 설정합니다.
- Storybook: @storybook/addon-styling 패키지를 추가하여 sass를 지원하도록 합니다.
- TypeScript: typescript-plugin-css-modules 플러그인 및 vscode에서 workspace의 ts를 사용하도록 변경합니다. 더 안전하게 CSS 모듈을 사용할 수 있도록 합니다.

CSS 모듈의 타입 체킹이 가능해집니다.

Details

자세한 내용은 셀프 코드 리뷰로 남깁니다.

Breaking change or not (Yes/No)

Yes

interpolation prop이 deprecated 됩니다.
SpinnerThickness enum이 deprecated 됩니다.

References

changeset-bot · 2023-07-11T13:12:31Z

⚠️ No Changeset found

Latest commit: fd00c0e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

github-actions · 2023-07-11T13:17:21Z

Chromatic Report

🚀 Congratulations! Your build was successful!

codecov · 2023-07-12T04:03:14Z

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.05 ⚠️

Comparison is base (14c4deb) 87.31% compared to head (fd00c0e) 87.27%.

Additional details and impacted files

@@                Coverage Diff                @@
##           styled-system    #1488      +/-   ##
=================================================
- Coverage          87.31%   87.27%   -0.05%     
=================================================
  Files                280      279       -1     
  Lines               3848     3835      -13     
  Branches             805      806       +1     
=================================================
- Hits                3360     3347      -13     
  Misses               415      415              
  Partials              73       73

Impacted Files	Coverage Δ
...zier-react/src/components/Spinner/Spinner.types.ts	`100.00% <ø> (ø)`
...es/bezier-react/src/components/Spinner/Spinner.tsx	`100.00% <100.00%> (ø)`

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

sungik-choi · 2023-07-12T06:10:25Z

packages/bezier-react/src/types/declaration/custom.d.ts

+declare module '*.module.scss' {
+  const classes: { [className: string]: string }
+  export default classes
+}


SCSS 모듈을 위한 전역 타입 선언을 추가합니다.

sungik-choi · 2023-07-12T06:10:53Z

packages/bezier-react/src/components/Spinner/Spinner.tsx

  size = SpinnerSize.M,
  color,
  ...rest
 }, forwardedRef) {
+  const Element = as || 'div'


styled-components의 as prop은 이러한 방식으로 직접 구현해주어야합니다.

sungik-choi · 2023-07-12T06:12:07Z

packages/bezier-react/src/components/Spinner/Spinner.tsx

+      style={{
+        '--bezier-spinner-color': color ? `var(--${color})` as const : undefined,
+        ...style,
+      }}
+      className={classNames(
+        styles.Spinner,
+        styles[size],
+        className,
+      )}


동적 스타일은 style, 정적 스타일은 className으로 구현합니다.

sungik-choi · 2023-07-12T06:15:08Z

packages/bezier-react/src/components/Spinner/Spinner.module.scss

+  height: $size;
+}
+
+@layer components {


@layer components 를 명시해줘야합니다.

라이브러리 내의 컴포넌트 스타일은 components layer 내부로 격리됩니다. 따라서 라이브러리 사용처에서 스타일 오버라이드 시, 라이브러리 내부의 선택자 우선순위에 대해 신경쓰지 않아도 됩니다.

sungik-choi · 2023-07-12T06:15:36Z

packages/bezier-react/.eslintrc.js

@@ -46,7 +46,7 @@ module.exports = {
            position: 'after',
          },
          {
-            pattern: './**/*.styled',
+            pattern: './**/*.+(styled|scss)',


import/order 규칙이 scss도 지원하도록 개선합니다.

sungik-choi · 2023-07-12T06:30:10Z

packages/bezier-react/src/components/Spinner/Spinner.test.tsx

+describe('Spinner >', () => {
+  const renderSpinner = (props?: React.ComponentProps<typeof Spinner>) => render(
+    <Spinner {...props} />,


jest & testing-library에서 css를(className은 가능) jsdom에 제대로 모킹하지 못하는 문제가 있습니다. 이를 해결하고자 리서치를 해봤지만 마땅한 해결책은 없었습니다.

#1330 에서 제안하는 유닛 테스트의 책임을 명확히하고, 테스트를 적절한 방식으로 변경하는 방향으로 문제를 해결하고자 했습니다. 즉, 상세한 스타일(css)에 대한 검증은 유닛 테스트에선 진행하지 않는 것입니다.

컴포넌트가 엘리먼트를 잘 렌더하는지

BezierComponentProps 인터페이스를 잘 준수하는지

클래스 네임이 의도대로 설정되는지

이 PR에선 위 3가지를 검증하기 위한 테스트 케이스로 변경했습니다.

이후 스타일에 대한 검증은 Chromatic 사용 방식을 개선하여 시각적 회귀 테스트를 작성하는 방향으로 진행해볼 예정입니다.

더 복잡한 테스트가 필요한 경우, Storybook의 Interaction test, A11y test 등을 적용해볼 예정입니다.

… classname-based styling (#1488) * chore(d.ts): add scss module declaration * style(eslint): add scss module pattern to eslint import order rule * test(story): sass environment setting * feat(spinner): change to classname-based styling * docs(component-props): deprecated interpolation prop * refactor(d.ts): change target module pattern * chore(storybook): delete mock style * refactor(spinner): rename component to element * chore(env): enhance to support CSS modules in TS * fix: delete unused ts interface * chore(styles): apply temporary relative path * style(storybook): apply tilde alias * fix(spinner): fix xl size * refactor(spinner): use mixin * test(spinner): rm test cases that don't work * test: update snapshot * test(spinner): enhance * test(spinner): add test case for forward ref * chore: change code owner * test(spinner): add test case for color prop

sungik-choi added #styled-system feat Issue or PR related to a new feature labels Jul 11, 2023

sungik-choi self-assigned this Jul 11, 2023

sungik-choi requested a review from quino0627 as a code owner July 11, 2023 13:12

sungik-choi commented Jul 12, 2023

View reviewed changes

sungik-choi force-pushed the styled-system branch from f9fd8e7 to 14c4deb Compare July 12, 2023 07:48

sungik-choi added 19 commits July 12, 2023 16:49

chore(d.ts): add scss module declaration

265c341

style(eslint): add scss module pattern to eslint import order rule

8ae951b

test(story): sass environment setting

ddb687f

feat(spinner): change to classname-based styling

00b7c82

docs(component-props): deprecated interpolation prop

767c2f8

refactor(d.ts): change target module pattern

c62cace

chore(storybook): delete mock style

d6f44a5

refactor(spinner): rename component to element

2d58f71

chore(env): enhance to support CSS modules in TS

90e089b

fix: delete unused ts interface

847b571

chore(styles): apply temporary relative path

fd169d2

style(storybook): apply tilde alias

4257259

fix(spinner): fix xl size

70f88bf

refactor(spinner): use mixin

3f1d6c8

test(spinner): rm test cases that don't work

d0ec719

test: update snapshot

a7524da

test(spinner): enhance

d0228eb

test(spinner): add test case for forward ref

2bbf08d

chore: change code owner

0cecad0

sungik-choi force-pushed the feat/layer-components branch from b0e0918 to 0cecad0 Compare July 12, 2023 07:49

test(spinner): add test case for color prop

fd00c0e

sungik-choi merged commit c406f6b into channel-io:styled-system Jul 13, 2023

sungik-choi mentioned this pull request Jul 13, 2023

Merge alpha <- styled-system #1492

Merged

sungik-choi mentioned this pull request Nov 16, 2023

Migrating component styling to Sass-based #1733

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up SCSS module environment(ts, storybook) and change `Spinner` to classname-based styling #1488

Set up SCSS module environment(ts, storybook) and change `Spinner` to classname-based styling #1488

sungik-choi commented Jul 11, 2023 •

edited

Loading

changeset-bot bot commented Jul 11, 2023 •

edited

Loading

github-actions bot commented Jul 11, 2023 •

edited

Loading

codecov bot commented Jul 12, 2023 •

edited

Loading

sungik-choi Jul 12, 2023

sungik-choi Jul 12, 2023

sungik-choi Jul 12, 2023

sungik-choi Jul 12, 2023 •

edited

Loading

sungik-choi Jul 12, 2023

sungik-choi Jul 12, 2023 •

edited

Loading

Set up SCSS module environment(ts, storybook) and change Spinner to classname-based styling #1488

Set up SCSS module environment(ts, storybook) and change Spinner to classname-based styling #1488

Conversation

sungik-choi commented Jul 11, 2023 • edited Loading

Self Checklist

Related Issue

Summary

Details

Breaking change or not (Yes/No)

References

changeset-bot bot commented Jul 11, 2023 • edited Loading

⚠️ No Changeset found

github-actions bot commented Jul 11, 2023 • edited Loading

Chromatic Report

codecov bot commented Jul 12, 2023 • edited Loading

Codecov Report

sungik-choi Jul 12, 2023

Choose a reason for hiding this comment

sungik-choi Jul 12, 2023

Choose a reason for hiding this comment

sungik-choi Jul 12, 2023

Choose a reason for hiding this comment

sungik-choi Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

sungik-choi Jul 12, 2023

Choose a reason for hiding this comment

sungik-choi Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Set up SCSS module environment(ts, storybook) and change `Spinner` to classname-based styling #1488

Set up SCSS module environment(ts, storybook) and change `Spinner` to classname-based styling #1488

sungik-choi commented Jul 11, 2023 •

edited

Loading

changeset-bot bot commented Jul 11, 2023 •

edited

Loading

github-actions bot commented Jul 11, 2023 •

edited

Loading

codecov bot commented Jul 12, 2023 •

edited

Loading

sungik-choi Jul 12, 2023 •

edited

Loading

sungik-choi Jul 12, 2023 •

edited

Loading