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

SegmentedControlの概要と使用上の注意を追加 #1141

Merged
merged 14 commits into from
Jun 18, 2024

Conversation

o-kwr
Copy link
Contributor

@o-kwr o-kwr commented Apr 22, 2024

課題・背景

https://smarthr.atlassian.net/browse/SD-635
https://smarthr.atlassian.net/browse/SD-576

基準充足!

やったこと

  • SegmentedControlの概要とどんなときに使うべきかを追加した

やらなかったこと

動作確認

Previewから確認お願いします!
https://deploy-preview-1141--smarthr-design-system.netlify.app/products/components/segmented-control/

キャプチャ

Before After

Copy link

netlify bot commented Apr 22, 2024

Deploy Preview for smarthr-design-system ready!

Name Link
🔨 Latest commit a401b95
🔍 Latest deploy log https://app.netlify.com/sites/smarthr-design-system/deploys/666fe4d713f2960009ec5b68
😎 Deploy Preview https://deploy-preview-1141--smarthr-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@o-kwr o-kwr marked this pull request as ready for review April 22, 2024 08:34
@o-kwr o-kwr requested review from versionfive, tosiiu and a team as code owners April 22, 2024 08:34
Copy link
Contributor

@versionfive versionfive left a comment

Choose a reason for hiding this comment

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

コメントしました!

@@ -5,8 +5,19 @@ description: ''
import { ComponentStory } from '@Components/ComponentStory'
import { ComponentPropsTable } from '@Components/ComponentPropsTable'

2つ以上の異なる状態を切り替えるためのコンポーネントです。[Switch](/products/components/switch/)はオンとオフの単純な2つの状態を切り替えるためのコンポーネントですが、SegmentedControlは任意の異なる状態を3つ以上持つことができます。

振る舞いとしては[RadioButton](/products/components/radio-button/)と同義とも言えますが、選択(状態の切り替え)によってビューに対して影響が発生することが特徴です。
Copy link
Contributor

Choose a reason for hiding this comment

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

概要文が大きく、読み切る必要があるので、この一文はRadioButtonとの使い分けになると思うので、使用上の注意に項目をつくってもよいのではと思いました。

Comment on lines 18 to 19
### ビューの関心が切り替わるときには用いない
異なる状態の切り替えによって、扱うオブジェクトごとビューの関心が変わる場合はSegmentedControlではなく[TabBar](/products/components/tab-bar/)を用いるようにしてください。
Copy link
Contributor

@versionfive versionfive Apr 22, 2024

Choose a reason for hiding this comment

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

このドキュメントはSegmentedControlの使い方を示すページなので、「こういうときに用いる」というスタンスで書けるならより適当かと思いました。

誤った使い方が多いから「このときは使わないで」を示す使い方もあると思いますが、SegmentedControlとTabBarに関しては「どっちを使うとよい?」という迷いが出発点なので、「こういうときにSegmentedControlを使ってね」中心に書けると良さそうだと思いました

Copy link
Collaborator

Choose a reason for hiding this comment

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

SegmentedControl も TabBar も、なんだったら SideNav などをも包括したパターンの可能性もある?

Copy link
Contributor

@oti oti left a comment

Choose a reason for hiding this comment

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

SegmentControl を使うにふさわしい、同じオブジェクトの状態を切り替えるために用いることがスッとわかる具体例が欲しいと思いました!

Copy link
Contributor

@tosiiu tosiiu left a comment

Choose a reason for hiding this comment

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

既出のコメントと同じく、近しいコンポーネントとの使い分けが迷うポイントだと思うので独立した章としてSwitchのように記載されているといいなと思いました!

Copy link
Member

@MasafumiKabe MasafumiKabe left a comment

Choose a reason for hiding this comment

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

既出のコメントと同じお気持ちです!
類似のコンポーネントとの違いなどについて割と序盤から言及しているので、まずは SegmentedContorol 単品について簡単に説明できていると良いと思いました!

Copy link
Contributor

@oti oti left a comment

Choose a reason for hiding this comment

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

いくつかコメントしました!

```

### Switch との違い
Switchはオン・オフのどちらか1つの状態を表します。対する概念ではあるが、異なる状態を表す場合はSegmentedControlを使ってください。
Copy link
Contributor

Choose a reason for hiding this comment

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

このページの主体はSegmentedControlなので主従が逆だと思いました。

e.g)SegmentedControlは〜〜です。〜〜の場合はSwitchを使用してください。

Switchはオン・オフのどちらか1つの状態を表します。対する概念ではあるが、異なる状態を表す場合はSegmentedControlを使ってください。

### RadioButton との違い
振る舞いとしては[RadioButton](/products/components/radio-button/)と同義とも言えますが、選択(状態の切り替え)によって即座にビューに対して影響が発生することが特徴です。絞り込み時にsubmitが発生する場合はRadioButtonの使用も検討してみてください。
Copy link
Contributor

Choose a reason for hiding this comment

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

うまく説明できているように感じなかったのと、「絞り込み時に」という言葉が急に出てきたのが引っかかりました。SegmentedControl か Tab か Switch か RadioButton かという話のなかに、絞り込みという認識はないように思います。言いたいことは「表示切替時に〜」なのかな?

Copy link
Contributor

@versionfive versionfive left a comment

Choose a reason for hiding this comment

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

前提の確認をSlackでしておりますー

## 使用上の注意

### TabBar との違い
ビューに対してフィルタリングしたり見え方を変えたいときに[TabBar](/products/components/tab-bar/)も用いることができますが、TabBarの場合はそれぞれのタブに内包されるオブジェクト自体が異なる場合に使うことが適しています。
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ビューに対してフィルタリングしたり見え方を変えたいときに[TabBar](/products/components/tab-bar/)も用いることができますが、TabBarの場合はそれぞれのタブに内包されるオブジェクト自体が異なる場合に使うことが適しています
ビューに対してフィルタリングしたり見え方を変えたいときに[TabBar](/products/components/tab-bar/)を使う場合がありますが、TabBarはそれぞれのタブに内包されるオブジェクト自体が異なる場合に使うことが適しています
一方、SegmentedControlはそれぞれのセグメントに内包されるオブジェクトが同一である場合に使うことが適しています。

書きっぷりはこれでいいのかわかりませんが、SegmentedControlはどうだ、も書いてほしいです。

Copy link
Contributor

@tosiiu tosiiu left a comment

Choose a reason for hiding this comment

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

切り替えの対象として「状態・フィルタリング・モード・見た目」が挙げられていますが、フィルタリングやモードの変化が何を示すのか明確に分かりませんでした...!何かしら説明があるとより分かりやすいと感じました

@o-kwr o-kwr requested review from oti, versionfive and tosiiu June 7, 2024 09:19
Copy link
Contributor

@versionfive versionfive left a comment

Choose a reason for hiding this comment

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

操作後の印象として、「オブジェクトが絞り込まれる(全体から一部が表示される)」というより「オブジェクトの一部が表示されたビューに切り替わる」ような結果になる気がしてきました。

また、mdxファイルのヘッダーのdescriptionが空なので埋めるようにお願いします。

Copy link
Contributor

@tosiiu tosiiu left a comment

Choose a reason for hiding this comment

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

v5-sanのコメントに同意です。
「ビューの切り替え」に、レイアウトの変更も表示項目の変更(絞り込み)も含まれていると解釈して整理しても良さそうと思いました。

o-kwr and others added 4 commits June 10, 2024 16:31
Co-authored-by: versionfive <64398878+versionfive@users.noreply.github.com>
Co-authored-by: versionfive <64398878+versionfive@users.noreply.github.com>
Co-authored-by: versionfive <64398878+versionfive@users.noreply.github.com>
@o-kwr o-kwr requested review from tosiiu and versionfive June 10, 2024 07:39
oti
oti previously approved these changes Jun 10, 2024
Copy link
Contributor

@oti oti left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@tosiiu tosiiu left a comment

Choose a reason for hiding this comment

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

一点コメントしましたが、大枠LGTMです!

content/articles/products/components/segmented-control.mdx Outdated Show resolved Hide resolved
MasafumiKabe
MasafumiKabe previously approved these changes Jun 10, 2024
Copy link
Member

@MasafumiKabe MasafumiKabe left a comment

Choose a reason for hiding this comment

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

yosasou~

uknmr
uknmr previously approved these changes Jun 10, 2024
Copy link
Collaborator

@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.

LGTM~
(「使用してください」と「使ってください」が揺れてるのはちょっと気になりました)

Comment on lines 21 to 31
<TabBar id="hover">
<TabItem id="border-1" selected>
書類
</TabItem>
<TabItem id="border-2">
承認経路
</TabItem>
<TabItem id="border-3">
承認履歴
</TabItem>
</TabBar>
Copy link
Collaborator

Choose a reason for hiding this comment

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

不要そう・名前不適切そう。

Suggested change
<TabBar id="hover">
<TabItem id="border-1" selected>
書類
</TabItem>
<TabItem id="border-2">
承認経路
</TabItem>
<TabItem id="border-3">
承認履歴
</TabItem>
</TabBar>
<TabBar>
<TabItem id="object-1" selected>
書類
</TabItem>
<TabItem id="object-2">
承認経路
</TabItem>
<TabItem id="object-3">
承認履歴
</TabItem>
</TabBar>

Copy link
Contributor

Choose a reason for hiding this comment

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

たしかに、TabBarのページではないので、ここになくてもよさそう

ytysym
ytysym previously approved these changes Jun 10, 2024
Copy link
Contributor

@ytysym ytysym left a comment

Choose a reason for hiding this comment

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

よさそうでした!

<ComponentStory name="SegmentedControl" />

## 使用上の注意

### ビューの関心が切り替わるときには用いない
Copy link
Contributor

Choose a reason for hiding this comment

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

・おおむねv5さんと同じ意見です。
「同じオブジェクトに対してビューを切り替える」はこのコンポーネントの挙動への期待として明確に含まれている認識なので、ここは強調して具体的な推奨パターンなど書いてあげてもよさそうだと思いました。

・以下の場合はNG側だと思うのですが、個人的に少し悩んだので考慮いただけるとよさそうかな〜と思いました。
コレクションビューで、オブジェクトのプロパティを起点に、表示するインスタンスを切り替える(ステータスやアーカイブのようなやつ)

・あと年末調整のこの使い方はNGかも?と思ったので、一応メモしておきます。
こういうNGを確実に弾ける基準、という考え方もありだと思うので。
image

@o-kwr o-kwr dismissed stale reviews from ytysym, uknmr, MasafumiKabe, and oti via 3a0852a June 14, 2024 04:35
Copy link
Contributor

@tosiiu tosiiu left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@oti oti left a comment

Choose a reason for hiding this comment

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

解説全体的に良くなったと思います!

見出しを「どういうときに使う/使わない」側の表現に揃えてみました。2つずつ提案しています。

<ComponentStory name="SegmentedControl" />

## 使用上の注意

### TabBar との違い
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### TabBar との違い
### オブジェクト自体を切り替える場合には使用しない
Suggested change
### TabBar との違い
### オブジェクト自体を切り替える場合はTabBarを使用する


オブジェクト自体を切り替えて表示する場合は、[TabBar](/products/components/tab-bar/)を使ってください。

### Switch との違い
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Switch との違い
### オン・オフを切り替える場合には使用しない
Suggested change
### Switch との違い
### オン・オフを切り替える場合にはSwitchを使用する

### Switch との違い
SegmentedControlは異なる状態を切り替えるためのコンポーネントですが、オン・オフのどちらか1つの状態を表して切り替えるときは[Switch](/products/components/switch/)を使ってください。

### RadioButton との違い
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### RadioButton との違い
### Submitが発生する操作の選択項目には使用しない
Suggested change
### RadioButton との違い
### Submitが発生する操作の選択項目にはRadioButtonを使用する

オブジェクト自体を切り替えて表示する場合は、[TabBar](/products/components/tab-bar/)を使ってください。

### Switch との違い
SegmentedControlは異なる状態を切り替えるためのコンポーネントですが、オン・オフのどちらか1つの状態を表して切り替えるときは[Switch](/products/components/switch/)を使ってください。
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
SegmentedControlは異なる状態を切り替えるためのコンポーネントですが、オン・オフのどちらか1つの状態を表して切り替えるときは[Switch](/products/components/switch/)を使ってください。
SegmentedControlは異なる状態を切り替えるためのコンポーネントですが、オンとオフの2つの状態を切り替えるときは[Switch](/products/components/switch/)を使ってください。

[nits]なんとなく冗長に感じましたが、元のままでも良い気はしました。

### RadioButton との違い
「複数の選択肢から1つを選ぶ」という振る舞いは[RadioButton](/products/components/radio-button/)と似ていますが、SegmentedControlは操作すると即座にビューに対して影響が発生することが特徴です。

選択操作後に「送信」や「保存」といった`submit`が発生する場合は、RadioButtonを使ってください。
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
選択操作後に「送信」や「保存」といった`submit`が発生する場合は、RadioButtonを使ってください。
選択操作後に「送信」や「保存」といったSubmitが発生する場合は、RadioButtonを使ってください。

[IMO] ここでいうsubmitはコードではなく処理の名称なので、code にしなくてもよさそうに感じました。イモです。

Copy link
Contributor

@versionfive versionfive left a comment

Choose a reason for hiding this comment

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

使用上の注意の見出しについてはoti sanのコメントを反映後に確認しますが、さきに細かい点だけコメントしました

### RadioButton との違い
「複数の選択肢から1つを選ぶ」という振る舞いは[RadioButton](/products/components/radio-button/)と似ていますが、SegmentedControlは操作すると即座にビューに対して影響が発生することが特徴です。

選択操作後に「送信」や「保存」といった`submit`が発生する場合は、RadioButtonを使ってください。
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
選択操作後に「送信」や「保存」といった`submit`が発生する場合は、RadioButtonを使ってください。
選択操作後に`送信``保存`といったtype属性が`submit`のボタンなどを押すことで入力内容を適用したい場合は、RadioButtonを使ってください。

#1168 で検討されている表記とあわせてみました。

Copy link
Contributor

@ytysym ytysym left a comment

Choose a reason for hiding this comment

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

otiさん、v5さんのコメントの部分が修正される前提で、ほかは気になるところなかったのでLGTMです!

@o-kwr o-kwr requested review from versionfive and oti June 18, 2024 03:56
Copy link
Contributor

@oti oti left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@MasafumiKabe MasafumiKabe left a comment

Choose a reason for hiding this comment

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

改めて LGTM!

Copy link
Contributor

@versionfive versionfive left a comment

Choose a reason for hiding this comment

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

LGTM!

@o-kwr o-kwr merged commit 3ed61ee into main Jun 18, 2024
5 checks passed
@o-kwr o-kwr deleted the add-guideline-segmentedcontrol branch June 18, 2024 04:39
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.

7 participants