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

Implement theme toggle(multi theme) feature #1690

Closed
Tracked by #1800
sungik-choi opened this issue Oct 20, 2023 · 1 comment · Fixed by #1756
Closed
Tracked by #1800

Implement theme toggle(multi theme) feature #1690

sungik-choi opened this issue Oct 20, 2023 · 1 comment · Fixed by #1756
Assignees
Labels
feat Issue or PR related to a new feature priority:A Issue that important and urgent

Comments

@sungik-choi
Copy link
Contributor

sungik-choi commented Oct 20, 2023

Description

테마 토글 기능을 구현합니다.

NOTE: CSS-in-JS에서 static CSS(classname) 기반으로 스타일 시스템이 변경된 이후 시작할 수 있습니다.

Reasons for suggestion

현재 CSS-in-JS(styled-components)에서는 BezierProvider 에 theme 객체를 넘기는 것으로 테마 스위칭을 구현할 수 있습니다. static CSS 기반으로 스타일링 방식이 변경됨에따라 이 기능에도 변경이 필요합니다.

Proposed solution

  • data attribute 혹은 className 기반으로 테마를 변경하는 기능을 구현합니다.
  • 현재 BezierProvider 를 중첩함으로써 특정 테마를 제한하는 동작도 마찬가지로 구현할 수 있습니다.
  • 채널 데스크 어플리케이션의 BezierProvider 에 구현되어 있는 테마 변경 로직을 일부 옮겨올 수 있습니다.
  • 자바스크립트로도 현재 테마(디자인 토큰 셋)에 접근할 수 있으면 좋겠습니다. static CSS 기반으로 변경되더라도, 어플리케이션에서 JS에서 현재 테마와 CSS 값들에 접근해야하는 경우는 여전히 존재하기 때문입니다.
  • 테마가 시스템 설정을 따르는 케이스에 대한 처리가 필요합니다. (perfers-color-scheme)

아래는 data attribute 기반으로 작성해본 간단한 예시 코드입니다.

const useTheme = () => {
  const [theme, setTheme] = useContext(ThemeContext)
  // 테마(토큰)과 테마 설정 함수를 반환
  return [theme, setTheme] 
}

// 이 훅이 반환하는 setter를 호출하면 DarkTheme, LightTheme 하위에서는 올바르게 동작하지 않았으면 좋겠음!
// console.error("현재 다크 테마로 고정되어 있습니다") 같은 식의 에러가 노출되면 좋겠다.
const BezierProvider = (themeToken?: DesignTokenSet) => {
  const [theme, setTheme] = useState(themeToken)
  
  // div를 렌더하지 않는 편이 좋을 수도 있음.
  // 1. Overlay 같은 케이스처럼 container를 지정
  // 2. radix-ui의 asChild 속성처럼 하위 엘리먼트에 data-bezier-theme 속성을 런타임에 주입
  return (
    <ThemeContext value={[theme, setTheme]}>
      <div data-bezier-theme={getThemeName(theme)}>
        { children }
      </div>
    </ThemeContext>
  )
}
const DarkTheme = () => (
  <ThemeContext value={[darkThemeTokenSet, () => { console.error("...") }]}>
    <div data-bezier-theme="dark">
      { children }
    </div>
  </ThemeContext>
)

const LightTheme = () =>  ( /* ... */ )

References

@sungik-choi sungik-choi added the feat Issue or PR related to a new feature label Oct 20, 2023
@sungik-choi sungik-choi changed the title Theme toggle [RFC] Implement Theme toggle feature Nov 10, 2023
@sungik-choi sungik-choi added the priority:A Issue that important and urgent label Nov 16, 2023
sungik-choi added a commit that referenced this issue Nov 22, 2023
<!--
  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 -->

Fixes #994 

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

베지어 디자인 시스템의 디자인 토큰 패키지인 bezier-tokens 패키지를 추가합니다.

> **디자인 토큰이 무엇인가요?**
> 
> Design tokens are a methodology for expressing design decisions in a
platform-agnostic way so that they can be shared across different
disciplines, tools, and technologies. They help establish a common
vocabulary across organisations. (from w3c dtcg)
> 
> - https://design-tokens.github.io/community-group/format/
> - https://m3.material.io/foundations/design-tokens/overview

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

### Note

- 여러 디자인 토큰 변환 라이브러리를 리서치해보았습니다. 사용자의 규모와 향후 업데이트 로드맵, 커스터마이즈 가능 범위 등을
살펴보았을 때, Style dictionary가 가장 적절하다고 생각하여 선택하였습니다.
- 디자인 토큰을 피그마에서 연동하기에는 현상황에서 어려웠습니다. 현재 피그마 Variables가 오픈베타여서 타이포그래피 등의
토큰 등을 지원하고 있지 않는 상황입니다. 또한 피그마 Variables의 등장으로 Token Studio같은 서드파티 플러그인을
사용하지 않기로 팀 내부에서 결정했기 때문에, 피그마 Variables의 스펙이 언제든지 추가되거나 변할 수 있다는 뜻입니다.
따라서 지금 피그마-소스 코드 변환기를 구현하는 건 시기상조라고 생각했습니다.
- 현재 작업중인 새로운 디자인 시스템에 토큰을 적용하지 않고, 기존의(프로덕션) 레거시 디자인 토큰을 적용했습니다. 정확히는
현재 bezier-react의 Foundation들을 디자인 토큰으로 분해했습니다(= 피그마에는 토큰으로 분류되지 않은 경우도
있습니다). 토큰 적용 & 정적 스타일링 방식으로 변경 -> 새로운 디자인 토큰 적용으로 단계를 나누어가기 위해서입니다.

### Build step

빌드는 간략하게 다음의 과정으로 이루어집니다.

1. JSON(Design token)을 cjs/esm/css 로 변환합니다.
2. 변환된 cjs/esm 의 엔트리포인트(index.js)를 만듭니다.
3. 타입스크립트 컴파일러를 통해 변환된 js 파일로부터 타입 선언을 만듭니다.

- **향후 1번의 변환 과정에 iOS, Android용 스타일 변환기, JSON 변환기 등을 추가할 수 있습니다.**
- 1번의 변환 과정은 글로벌 토큰(기존의 팔레트, 레디우스 등)과 시맨틱 토큰(라이트/다크 테마)이 별개로 이루어집니다.
라이트/다크 테마를 함께 빌드하게 되면 키가 충돌했다는 메세지와 함께 빌드 에러가 발생합니다. themeable같은 속성을 사용할
수도 있으나, JSON에 style-dictionary 라이브러리에 종속적인 속성을 포함시키고 싶지 않았습니다. 토큰은 더
순수하게 두는 게 나중을 위하여 좋다고 판단했습니다.
- Composite token(예: 타이포그래피)를 지원하지 않습니다. 현재 공식적으로 지원하지 않는 스펙이며, 현상황에서는
개별 토큰들을 bezier-react(그 외 각 플랫폼 디자인 시스템)에서 조합해도 큰 무리가 없다고 판단했습니다.

#### File tree

```md
dist
 ┣ cjs
 ┃ ┣ darkTheme.js
 ┃ ┣ global.js
 ┃ ┣ index.js
 ┃ ┗ lightTheme.js
 ┣ css
 ┃ ┣ dark-theme.css
 ┃ ┣ global.css
 ┃ ┗ light-theme.css
 ┣ esm
 ┃ ┣ darkTheme.mjs
 ┃ ┣ global.mjs
 ┃ ┣ index.mjs
 ┃ ┗ lightTheme.mjs
 ┗ types
 ┃ ┣ cjs
 ┃ ┃ ┣ darkTheme.d.ts
 ┃ ┃ ┣ darkTheme.d.ts.map
 ┃ ┃ ┣ global.d.ts
 ┃ ┃ ┣ global.d.ts.map
 ┃ ┃ ┣ index.d.ts
 ┃ ┃ ┣ index.d.ts.map
 ┃ ┃ ┣ lightTheme.d.ts
 ┃ ┃ ┗ lightTheme.d.ts.map
 ┃ ┗ esm
 ┃ ┃ ┣ darkTheme.d.mts
 ┃ ┃ ┣ darkTheme.d.mts.map
 ┃ ┃ ┣ global.d.mts
 ┃ ┃ ┣ global.d.mts.map
 ┃ ┃ ┣ index.d.mts
 ┃ ┃ ┣ index.d.mts.map
 ┃ ┃ ┣ lightTheme.d.mts
 ┃ ┃ ┗ lightTheme.d.mts.map
```

### Next

- 이 패키지의 js, css를 가지고 bezier-react의 스타일 시스템, 테마 기능을 구성하게 됩니다. (#1690)
- 이 패키지의 토큰에 더해 bezier-react의 constants(disabled 0.4, z-index), 타이포그래피
등을 bezier-react에서 추가, 확장하여 최종적으로 사용자 애플리케이션에 제공하는 방향으로 구현하고자 합니다. (#1495
에서 작업)

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

No

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

- https://amzn.github.io/style-dictionary
- https://dbanks.design/blog/dark-mode-with-style-dictionary/
- amzn/style-dictionary#848 : Composite token
관련 이슈
@sungik-choi sungik-choi self-assigned this Nov 22, 2023
@sungik-choi
Copy link
Contributor Author

@sungik-choi sungik-choi changed the title Implement Theme toggle feature Implement theme toggle(multi theme) feature Nov 23, 2023
sungik-choi added a commit that referenced this issue Dec 1, 2023
<!--
  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)
- [ ] 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 -->

Fixes #1690

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

data attributes를 기반으로 한 멀티 테마 기능을 구현합니다.

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

foundation 객체를 주입받는 형식에서, themeNam 문자열을 주입받아 하위 엘리먼트(루트 엘리먼트가 되길 기대)의
data attribute를 변경 & 해당하는 토큰 객체를 컨텍스트로 전달하는 AppProvider를 구현합니다.

- features prop: 사용 편의성을 위해 `FeatureProvider` 를 빌트인으로 가지는 방향으로 변경했습니다.
기본값은 빈 배열입니다.
- BezierProvider to AppProvider: 앱의 루트에 적용하는 Provider라는 점을 강조하기 위해
Bezier 대신 App이라는 접두어를 붙였습니다.

라이트테마, 다크테마 혹은 테마 역전(Tooltip 등)이 필요한 곳에서 테마를 고정해서 사용할 수 있는
ThemeProvider를 구현합니다. 이제 `--inverted-` 토큰이 사라지는 대신, 하위 엘리먼트에 radix-ui
Slot을 통해 data theme attribute를 전달하여 토큰을 스위칭하는 방식으로 동작하게 됩니다.

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

No. 기존 BezierProvider는 그대로 유지합니다.

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

- https://www.radix-ui.com/themes/docs/components/theme
- https://panda-css.com/docs/guides/multiple-themes
-
https://github.com/Shopify/polaris/blob/main/polaris-react/src/components/AppProvider/AppProvider.tsx
@sungik-choi sungik-choi mentioned this issue Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Issue or PR related to a new feature priority:A Issue that important and urgent
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant