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

Palette型のプロパティを再定義する #872

Open
2 tasks
youchann opened this issue Jun 23, 2022 · 4 comments
Open
2 tasks

Palette型のプロパティを再定義する #872

youchann opened this issue Jun 23, 2022 · 4 comments
Assignees
Milestone

Comments

@youchann
Copy link
Contributor

youchann commented Jun 23, 2022

背景

現在のPalette型の定義はこのようになっている。

export type Palette = {
  white: string;
  black: string;
  primary: PaletteColor;
  success: PaletteColor;
  warning: PaletteColor;
  danger: PaletteColor;
  gray: PaletteColor;
  text: PaletteText;
  background: PaletteBackground;
  divider: string;
  icon: PaletteIcon;
};

このような定義型は下記のような問題点を孕んでいる。

  • 保持しているプロパティが多い
    • ingred-uiの利用者は新規追加するコンポーネントに対してどの色を割り当てるかの判断コストがかかる
  • プロパティ名が用途ではなく色となっているものがある(e.g. Palette["gray"])
    • Themingが難しくなる
      • 例えば任意の箇所の背景を変えたい場合に意図しない箇所のテキスト色が変わる可能性がある

他の社内OSSやコンポーネントライブラリを参考にし、適切なPalette型を作成する。

TODO

  • Palette型を再考する
  • 新たな型をingred-uiに適用する
@youchann youchann added this to To do in INGRED UI Roadmap via automation Jun 23, 2022
@youchann youchann added this to the TEST milestone Jun 23, 2022
@keisukeonishi
Copy link
Contributor

コードをもとに現在paletteで定義されている色をXDで洗い出してみました。
各色の数字のズレもありつつ、同じグレーが使われていたり白が直接#ffffffと打たれちゃってたりしてますね
わりとメチャのクチャな感じがします

components – COLOR PALETTE

@youchann
Copy link
Contributor Author

@takurinton

#940
↑あたりが関係あるやつかな?

どういう対応をとって、いまどうなっているのかを書いてほしいです:pray:

@takurinton
Copy link
Contributor

takurinton commented Jan 4, 2023

プロパティ名が用途ではなく色となっているものがある(e.g. Palette["gray"])

この箇所について、gray を全て綺麗に定義することは現状の構成だと難しそうだったので、gray が使われてる中でも box-shadow で使われてる gray にフォーカスを当てて対応をした。

関連PR

@takurinton
Copy link
Contributor

takurinton commented Jan 4, 2023

gray の部分に関しては、今の所 getShadow 関数を使ってトークン番号を渡すとその shadow を返してくれるようになっている。
トークン番号は #940 (comment) で定義されている通り。

補足
本来、shadow は palette の中で定義したかったが、palette の中で定義しなかった理由として一部動的に値を指定したいトークンが存在したというものがある。

@youchann youchann removed their assignment May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants