-
Notifications
You must be signed in to change notification settings - Fork 304
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
エンジンの管理ダイアログのリデザイン #2255
エンジンの管理ダイアログのリデザイン #2255
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほぼ lgtm です!!!
デザインありがとうございます、素敵だと思います!!!
いくつか細かい点をコメントさせていただきました!
[data-state="off"] > .check { | ||
display: none; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
チェックしたかしてないかで横の長さが変わるのは、もしかしたらユーザーが予測できないかもと思いました!
ボタングループ内のボタンが2種だけだったら問題なさそうですが、仮に3つあって左から順にボタンをクリックしようとした時、ボタンの横の長さが成長して間違えて前のボタンをクリックしてしまうかも。
アクセシビリティ的に色だけじゃなくて形も変わった方が良い、という気持ちなのかなと思いました!
良い解決策が思いついてるわけじゃないのですが、空白を表示しておくか、チェックマークが入ってもサイズが変わらない十分な横幅を確保するか、とか・・・・・・?
あるいはチェックマークを表示しないオプションがあれば良いかも!
というか問題が発生してから考えるでもいいかも!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
幅が変わることに関してはどうしても残すべきというほどのメリットはなさそうです…!
個人的に色以外でもどれが選択されてるかを示すものは欲しいと思うのでチェックマークはあるまま、幅が変わらないようにチェック時にマークの幅が増えた分padding値を減らして相殺する形にしました。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なるほどです!一旦良さそう!!
ちょっと今すぐどうしてほしいということはないのですが、お伝えまで 🙏
ソング側でも @romot-co さんからトグルボタングループのUIが提案されていて、そちらはどちらかというと枠を追加して形を変える感じだったりします。
特に ソング UI のメイン画面はものすごい多機能になっていき、ボタンが大量に必要になってくるので、チェックマークを表示するスペースがもったいないので、少なくともソングUIのこのボタンに関しては、この形が良いように感じてます。
ちなみにMUIもそんな感じの見た目でした。
https://mui.com/joy-ui/react-radio-button/#segmented-controls
ちなみにMaterial Design 3のサンプルは選択チェックマーク式。
https://m3.material.io/components/segmented-buttons/overview
どちらが優れているとかはないのですが、もしかしたら将来的に見た目を揃えられるといいのかもとちょっとだけ思いました!
ただまぁアイコンがある場合は選択中のボタンに枠を設ける形に、テキストだけしかない場合はチェックマークを表示するみたいな使い分けもできると思うので、あまり気にしてないです。
(強いて言うなら、おそらくクリエイター向けソフトで何度も実行される場所のUIは、動きが無い方が好ましいかもくらい。)
こういう違いがあるよ~というのを、とりあえずお2人にお伝えしたかった次第です 🙏
border: 1px solid; | ||
transition: background-color vars.$transition-duration; | ||
cursor: pointer; | ||
box-shadow: 0 1px 2px rgba(0, 0, 0, 0.05); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(ただのコメントです)
shadow系の色、そろそろCSS変数にまとめていっちゃっても良いかもですね!
<div class="list-header"> | ||
<div class="header-title">エンジン一覧</div> | ||
<BaseButton | ||
label="追加" | ||
icon="add" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これちょっと迷ってます 😇
ちょっと不本意ですが、ここのボタンだけborderの色を濃くさせていただくかもです 🙇
案はいくつかあるのですが、それぞれちょっと問題がありそう。
- エンジン一覧ダイアログに来た時、右側の空いてるスペースにも「エンジンを追加する」ボタンを大きく表示する
- 一度エンジン一覧から何かのエンジンを選択するとその表示が消えちゃうので、ちょっと良くないUI。
- エンジンサブメニューの中に「エンジンを追加」項目を追加する
とりあえずこのプルリクエスト的には、マージするのに障害になりきらないので、未解決で良さそう!
<BaseButton | ||
label="再起動" | ||
icon="refresh" | ||
:disabled=" | ||
uiLocked || engineStates[selectedId] === 'STARTING' | ||
" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(判断メモです)
モーダルに入った後、最終的な操作をするボタンの横にアイコンを表示するの、良いのかどうか迷ったのですが良さそうだなと思いました!!
迷った理由は単純に、ここのボタンは任意の操作がくるので、アイコンが設定できないような複雑なものもあり、一貫性が下がると思ったためです。
一方でアイコンがあった方が絵になるし、あとこのソフトを使う人の気持ちもちょっと高まると思うので、設定できるところは設定するという方針もありだと感じました。
このトレードオフを考えて、まあスペースに余裕のあるところでアイコンもありそうなところは付けるのがいいのかなと思いました!
あ、もし何か参考にした考え方とかあったらお聞きしたいです 🙏
多分将来複雑な操作をするボタンと、アイコン設定できるボタンが入り組んだタイミングでまた迷いそうなので・・・!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
瞬時に認識できるようになるべくアイコンは入れていこうという個人的な考えでした!
まだ複雑な例にあたってないのでアイコンが設定できないものとの一貫性は考えてなかったですが、たしかにそうですね…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
別の調べてたらadobeに、ボタン内のアイコンどうするかの哲学があったので共有まで!
必要なもののみにアイコンを入れる(装飾には使わない)
https://spectrum.adobe.com/page/button/#Use-icons-only-when-necessary
優先度の高いボタンにアイコンがない場合は、優先度の低いものにはアイコンをつけない
https://spectrum.adobe.com/page/button-group/#Use-icons-only-for-the-most-critical-actions
とのことでした。真似るかどうかはさておき、参考になりそうなのでメモ。
position: absolute; | ||
z-index: 10; | ||
inset: 0; | ||
z-index: 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(ただのコメントです)
ここのz-indexも後々全体管理できるといいかもですね!
というよりそもそもここは、エンジン追加しようとした時に右側ペイン内に追加 UI を表示して、左のドロワー部分をdisableにするために導入されてるのですが、モーダルにしちゃっても良いかもとか思いました。
あまりドロワー部分のエンジンリストが表示され続けてる意味がなさそう。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほぼLGTMです!!
#2255 (comment) のコメント追加だけお願いできると・・・!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
素敵なデザインをありがとうございます!!!
Storybook実装もありがとうございます、レビューしやすくてめちゃくちゃ助かります!!
内容
エンジンの管理ダイアログをリデザインします。具体的に以下を行います。
スクリーンショット・動画など