search

uyupun / official

サグラダ・ファミリア
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Textコンポーネントにline-heightとfont-sizeを指定するpropsを追加 #379

Closed takashi0602 closed 7 months ago

takashi0602 commented 7 months ago

概要

Textコンポーネントにline-heightとfont-sizeを指定するpropsを追加しましたのでご確認よろしくお願いします。

背景

背景として PR #377 でTextコンポーネントのスタイルが優先されたため、デフォルトのスタイルを排除し、使用されるスタイルはすべてPropsとして渡すようにしました。

スクリーンショット

StorybookでTextコンポーネントを表示している。
uyupunpopunpo commented 7 months ago

Storybook URL: https://uyupun.github.io/official/feat_text-component-props/storybook/ Lighthouse URL: https://uyupun.github.io/official/feat_text-component-props/lighthouse/

tyokinuhata commented 7 months ago

これって実装的になんとかしてコンポーネントに渡したスタイルが優先されるようにするのは難しかったってこと？

tyokinuhata commented 7 months ago

このままどんどんpropsが増えていくとTextコンポーネントの旨味ってなくなっていくよなあ、と思いまして

takashi0602 commented 7 months ago

@tyokinuhata

レビューありがとうございます！

これって実装的になんとかしてコンポーネントに渡したスタイルが優先されるようにするのは難しかったってこと？

ですね... スタイルがclassに記述された順ではなく、cssファイルに記述された順で適応されていくので...💦 そのあたりvanilla-extractがうまく吸収してくれないかな〜とか思ったけど、無理そうっす

このままどんどんpropsが増えていくとTextコンポーネントの旨味ってなくなっていくよなあ、と思いまして

これ以上増えることはないと思う + BaseコンポーネントなんでPropsは多くなりがちなので妥協かなと...🥺

tyokinuhata commented 7 months ago

まあちょっとこれ以上propsが増えたら設計考え直しますか