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

fix(smarthr-ui-preset): breakpointを設定 #4965

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Qs-F
Copy link
Contributor

@Qs-F Qs-F commented Sep 30, 2024

関連URL

https://kufuinc.slack.com/archives/C013KCVC0P3/p1727686839153149

強い気持ちがある人が現れると修正内容がかわるかも https://kufuinc.slack.com/archives/CGC58MW01/p1727691651343259

概要

smarthr-ui-presetに対してbreakpointの設定を行った

変更内容

  • smarthr-ui-presetにbreakpointの設定を追加
    • createBreakpoint と同じ値を設定
    • 通常Tailwind CSSでは min-width で設定されるが、SmartHRではデスクトップを先に作ってからモバイル対応という流れが多いため、 SP:shr-hogehoge でモバイル用のスタイルをあてられる方が利点が高いと考え max-width で追加

確認方法

例えばButton.tsxのwrapperに SP:shr-bg-[red] を追加し、ブラウザの幅を縮めると背景色が赤くなることを確認

2024-09-30.19.05.47.mov

@Qs-F Qs-F marked this pull request as ready for review September 30, 2024 10:07
@Qs-F Qs-F requested a review from a team as a code owner September 30, 2024 10:07
@Qs-F Qs-F requested review from s-sasaki-0529 and hiroki0525 and removed request for a team September 30, 2024 10:07
Copy link

pkg-pr-new bot commented Sep 30, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/kufu/smarthr-ui@4965

commit: dacb0d3

Copy link
Contributor

@uknmr uknmr left a comment

Choose a reason for hiding this comment

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

SmartHR においては基本的にプリミティブレイアウトで対応してきたため、現状のメディアクエリトークンは活用されていない認識でした。
値としても大雑把で、isNarrowView のような使い方しかできないように見えるため、デザイントークンの再定義をしてから設定した方が良いと思いました。

@Qs-F
Copy link
Contributor Author

Qs-F commented Oct 8, 2024

議論の結果、 narrow トークンを追加するのでどうだろう、という話になったのでその方向で実装します

https://kufuinc.slack.com/archives/C013KCVC0P3/p1728375123717259?thread_ts=1727686839.153149&cid=C013KCVC0P3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants