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

feat(spindle-ui): create SegmentedControl #731

Merged
merged 1 commit into from Jun 28, 2023
Merged

Conversation

yu-3in
Copy link
Contributor

@yu-3in yu-3in commented Jun 20, 2023

概要

SegmentedControlコンポーネントを作成しました。SegmentedControlは、ページ内で機能やモードを切り替える際に利用するコンポーネントで、基本機能はラジオボタンと同様です。

デザイン

Size

Medium Large
スクリーンショット 2023-06-21 午後2 04 04 スクリーンショット 2023-06-21 午後2 04 09

動作確認

2023-06-21.2.08.46.mov

詳細

詳細な仕様に関しては、Experimental Features docsとAccessibility要件定義docsにまとめております。

TODO

  • バンドルサイズの maxSize を調整

@yu-3in yu-3in force-pushed the feat/segmented-control branch 2 times, most recently from f9307a3 to a89951c Compare June 20, 2023 05:44
@yu-3in yu-3in changed the title feat: create SegmentedControl (Draft) feat(spindle-ui): create SegmentedControl Jun 20, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 20, 2023

Visit the preview URL for this PR (updated for commit bfc63dc):

https://ameba-spindle--pr731-feat-segmented-contr-dnq536p6.web.app

(expires Fri, 28 Jul 2023 05:45:57 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: e7521619a2dd5c653490c8246e81ec2a5c8f1435

@reg-suit
Copy link

reg-suit bot commented Jun 20, 2023

reg-suit detected visual differences.

Check this report, and review them.

⚪⚪⚪⚪⚪⚪⚪⚪

🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵

What do the circles mean? The number of circles represent the number of changed images.
🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items

How can I change the check status? If reviewers approve this PR, the reg context status will be green automatically.

Copy link
Contributor

@itsminadesu itsminadesu left a comment

Choose a reason for hiding this comment

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

他に影響がありそうなところだけ先にコメントしました! 🙌🏻

(Draftなので、細かいところは見ていません 🙏🏻 )

@yu-3in
Copy link
Contributor Author

yu-3in commented Jun 21, 2023

バンドルサイズが大きいことが原因でテストに失敗しているようです。

スクリーンショット 2023-06-21 午後1 53 49

PaginationやDropdownMenuなどと同様に bundlesize.config.json にて maxSize1.6 kBに変更する方針で対応してもよろしいでしょうか?

@yu-3in yu-3in marked this pull request as ready for review June 21, 2023 05:15
@itsminadesu
Copy link
Contributor

@yuuumiravy
#731 (comment)

対応はそちらで大丈夫です! 🙌🏻
ただ、レビューを経て色々修正した結果sizeが変わることもあるので、マージ直前に対応で大丈夫です 🙆🏻‍♀️

Copy link
Contributor

@itsminadesu itsminadesu left a comment

Choose a reason for hiding this comment

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

一旦、Storybookの部分に関してだけコメントしました! 🙏🏻

(テンプレートのようなものがまだ整備されておらずすみませんが何卒... 🙇🏻 )(DesignDocsの整備済んだら、こちらもいずれ作りますか...! @herablog

Copy link
Contributor

@itsminadesu itsminadesu left a comment

Choose a reason for hiding this comment

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

スタイル部分みました! 🙏🏻

Copy link
Contributor

@tokimari tokimari left a comment

Choose a reason for hiding this comment

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

なにとぞ!:pray:

Copy link
Contributor

@itsminadesu itsminadesu left a comment

Choose a reason for hiding this comment

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

ロジックみました!

@yu-3in
Copy link
Contributor Author

yu-3in commented Jun 23, 2023

@tokimari
#731 (comment)

JSを使わないシンプルな方法でとても良いと思いました!ありがとうございます!

そちらの方法で改善しました:0e434b5

@itsminadesu itsminadesu self-requested a review June 26, 2023 09:46
Copy link
Contributor

@itsminadesu itsminadesu left a comment

Choose a reason for hiding this comment

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

一点だけ...! 🙏🏻
それ以外は良さそう...な気がします.....!!!

Copy link
Contributor

@itsminadesu itsminadesu left a comment

Choose a reason for hiding this comment

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

🎉🎉🎉
Spindle 使っていれば LGTM

Copy link
Contributor

@tokimari tokimari left a comment

Choose a reason for hiding this comment

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

すみません1点だけ見落とし・・:pray:
あとはよさそうです! 😄

@yu-3in
Copy link
Contributor Author

yu-3in commented Jun 28, 2023

Accesibirlityチェックが不完全な項目があったため対処しました:a06842d

協議の結果、font-weightのアニメーションはつけない方針となりました。

スクリーンショット 2023-06-28 午前10 07 29

@yu-3in
Copy link
Contributor Author

yu-3in commented Jun 28, 2023

再度全体を見直していく中でいくつか修正・リファクタリングしています。

  • リファクタリング:2e5ed5f(不要な条件を削除)
  • 1550cb7(デザインの要件と異なっていた)
  • Design Docs
    • アニメーショントークンの反映漏れ修正
    • 実装例の関数等の名前が間違っていたのを修正

Copy link
Member

@herablog herablog left a comment

Choose a reason for hiding this comment

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

エンジニアを褒めるネコ:優勝した

Copy link
Contributor

@tokimari tokimari left a comment

Choose a reason for hiding this comment

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

遅くなってごめんなさい:pray:

Copy link
Contributor

@tokimari tokimari left a comment

Choose a reason for hiding this comment

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

:otukaresamadesita: 🍻 :kakeru_kanpai:
エンジニアを褒めるネコ:リリースしました

@yu-3in yu-3in force-pushed the feat/segmented-control branch 3 times, most recently from 39b5ff5 to 7b88e00 Compare June 28, 2023 05:34
@yanagi0602
Copy link
Contributor

yanagi0602 commented Jun 28, 2023

https://github.com/openameba/spindle/pull/731/files#diff-5e020f4991e219ee13adced8ea4cacc0f49caaf454c45da5601024b701391625R136-R138
https://github.com/openameba/spindle/pull/731/files#diff-5e020f4991e219ee13adced8ea4cacc0f49caaf454c45da5601024b701391625R49-R51
https://github.com/openameba/spindle/pull/731/files#diff-a5f4a9907daafb0d56657233c1e1015b95732c07e71bb66fca09539eb93b45b2R7

このあたりの gap の4を定数化して、gapの値もinline styleで設定しても良いかも?と一瞬思ったものの、変更が加わるときには 4 という数値をイジるだけじゃなくいろんな微調整が発生すると思うのでこのままでも良いかも。という結論に至りました。

@yu-3in
Copy link
Contributor Author

yu-3in commented Jun 28, 2023

#731 (comment)

変更が加わるときには 4 という数値をイジるだけじゃなくいろんな微調整が発生すると思うのでこのままでも良いかも。

現時点では、以下の部分を修正する必要があります。

  • .spui-SegmentedConrolgap
  • .spui-SegmentedControl-dividergap
  • .spui-SegmentedControl--divider .spui-SegmentedControl-button::beforeleft
  • tsxの以下コードの4の部分(2箇所存在する点に注意)
`translateX(calc(${100 * selectedIndex}% + ${
  (4 + 4) * selectedIndex + 4
}px))`,

@herablog
Copy link
Member

いちお CSS カスタムプロパティ で解決できるかなぁとは思うんですが、u○sで動くか不明なんですよね・・

@yu-3in yu-3in merged commit e279439 into main Jun 28, 2023
11 of 12 checks passed
@yu-3in yu-3in deleted the feat/segmented-control branch June 28, 2023 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants