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.

Sign up for GitHub

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

Jump to bottom

ショートカットキー割り当てダイアログのリデザイン #2348

Merged
merged 26 commits into from
Dec 7, 2024
Merged

ショートカットキー割り当てダイアログのリデザイン #2348

merged 26 commits into from
Dec 7, 2024

Conversation

takusea
Copy link
Contributor

@takusea takusea commented Nov 9, 2024 •
edited
Loading

内容

ショートカットキー割り当てダイアログのリデザインを行います。

  • BaseIconButtonのコンポーネントとStoryファイルを追加
  • 未割り当てにするボタンを個別設定ダイアログ側から一覧側に移動
  • 表記揺れを統一
    • 未割り当て・未設定→未割り当て
    • デフォルト・初期値→デフォルト
  • すでに未割り当て・デフォルト状態のときに対象のボタンがdisabledになるようにする

スクリーンショット・動画など

image

@takusea takusea requested a review from a team as a code owner November 9, 2024 00:16
@takusea takusea requested review from Hiroshiba and removed request for a team November 9, 2024 00:16
@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Nov 9, 2024 •
edited
Loading

🚀 プレビュー用ページを作成しました 🚀

更新時点でのコミットハッシュ：6a2a7cb

@Hiroshiba
Copy link
Member

ありがとうございます！！！
ちょっと課題がいくつかありそうだったので、一旦コメントまで 🙇

  • 一覧性がもっとほしいかも
    • すでにショートカットキーはかなりの量があり、もう少し簡単にたどり着けるようにしたい
    • ちなみにVSCodeのショートカットキー一覧を参考にしたりしてます
      • image
    • 例えばひとつひとつのセルの縦幅を小さくするとか･･････？
      • あるいはテーブルにするか･･････？
      • その要素が何なのか説明したいからテーブルのがありがたい気もする
        • 「操作」とか「ショートカットキー」とか
      • 大変だったら、密度が高いBaseCellでも！
  • ショートカットキーをどう変更すればいいか一瞬迷うかも
    • 右にある「E」などのボタンが、その操作のショートカットキーを意味してることが若干わかりづらいかも
    • 多分理由が2つ、これがショートカットキーだという説明がないことと、常時表示の「未割り当てにする」等のボタンと重要度が同じに見えること（ショートカットキーの方が重要）
    • マウスホバーしないと「未割り当てにする」ボタンを出してないのはそういう理由でした！
    • 提案としては････ちょっと良い方法思いついてないです･････。
  • ゴミ箱ボタンが何をするか分からない
    • おそらく「操作を消す」に感じるはず
      • 正しくは「ショートカットキーを消す」
    • 削除ボタンが項目についてるため
    • これは以前たしかものすごく悩んで、とりあえずショートカットキー変更をしようとした時に未割り当てにできるようにしました 😇
    • VSCodeは右クリック→未割り当て（キーバインドの削除）でした。そういうUXもアリかも

実装コストとかもあると思うので、難しそう等あれば何でもコメントいただければ･･･！！ 🙇

@takusea
Copy link
Contributor Author

takusea commented Nov 11, 2024

image
一番簡単に修正する場合を想定してBaseRowCardの縦幅縮める・アイコンボタンを非ホバー時に消えるように戻すだと、上記のような感じ？一覧性は増しましたが、アイコン非表示時にカードだと右端の余白が気になりますね…

ボタンの左側に置くことも考えましたが、キーボードでのTab移動時に非表示だった要素にいきなりフォーカスがあたって違和感があるかも。やはり最高を目指すならテーブルにするのが良いかもですね…！

未割り当てのアイコンも…deleteではなくclear感のあるアイコンがあれば良さそうですが、Material Iconsにはなさそうでした。右クリックメニューに置くのは他にそれが必要な箇所が少ないので見つけづらいかも？

イラスト3
もしくはこんな感じでSplitButtonっぽくすれば項目自体の削除感は減りそう？ただここでしか使わないとなるとこの形のUIがボタンだと覚えづらいかも。

とりあえずテーブル寄りのデザインも試して見ようと思います…！

@Hiroshiba
Copy link
Member

Hiroshiba commented Nov 12, 2024 •
edited
Loading

完全におっしゃる通りだと思います 😇
（余白が気になる・tab移動で混乱する・右クリックは見つけづらい等）

キーボード操作のことは考えてませんでした。。。
確かにホバーで現れるのは避けた方が良い寄りではありますね。。。

キーとキーの削除ボタンをくっつけて並べた場合は兄弟になるので、親子構造には感じないかも･･･。
そういえばスマホとかの入力欄は×押すと入力内容はクリアされるボタンとかありますね！！
ギリギリあれだったら親子構造感あるかも。

とりあえずテーブル寄りのデザインも試して見ようと思います…！

ありがとうございます！！！

ホバーでボタンが現れないようにしてずらっとボタンを並べると、可視性はかなり下がるだろうと感じてます！
なんかこう、右クリックしたくなるような感じが出せると良さそうなのですが･･･････。

ちなみにVSCodeのショートカットキー設定はホバーした時に一番左に編集ボタンが出るのですが、キーボード操作の時はそのレーンがアクティブになっている時に現れるようになってました！
image
レーンをアクティブにする→操作（enterで編集開始、右クリックでいろいろ出てくる）という操作でなるほどでした。

@takusea
Copy link
Contributor Author

takusea commented Nov 17, 2024

テーブル寄りでのデザインを作ってみました。従来のものよりはセル高さが新コンポーネントに合わせて大きめですが、一覧性はカードのものよりは良さそうです。加えてテーブルの行（の背景）を幅いっぱいにすることで非ホバー時の余白余りは感じられなくなってそうです。
image

未割り当てボタンは項目の右端じゃ無くせば項目自体の削除感は薄れるかも？と思いデフォルトに戻すボタンの左に移してみてます。

右クリックできそうな感じ、なかなか難しそうですね…！表示で誘導するとまたメインのキー変更ボタンが埋もれそうで。ユーザーが見つけられるのを当てにするかどうか…！

@Hiroshiba
Copy link
Member

Hiroshiba commented Nov 17, 2024 •
edited
Loading

おーーーー！！！
一覧性もあるし、以前よりもショートカットキーをクリックできそう感があって、良さそう！！！

右クリックが難しいの、なるほどです。
とりあえずなし････ですかね･･･････。

あるいはショートカットキーをクリックした後に未割り当てとキーを削除ボタンを用意するとか･･･？
（まあこれは今までの形と似てますが）

削除アイコン

アイコンを左に、なるほどです。確かにちょっと変わりそう･･･？

アイコンですが、多分消しゴムアイコンが一番わかりやすそうな気がしました！！
その項目を消すのではなく、書いている内容が消える感じがするので。
でも調べてみた感じ、多分消しゴムアイコンを見ても消しゴムだとわからなさそうな見た目でした。。。

似たのを探してみました。この辺りですかねぇ。。。（Google Iconですが）

backspce
https://fonts.google.com/icons?selected=Material+Symbols+Outlined:backspace:FILL@0;wght@400;GRAD@0;opsz@24&icon.query=delete&icon.size=24&icon.color=%235f6368

variable_remove
https://fonts.google.com/icons?selected=Material+Symbols+Outlined:variable_remove:FILL@0;wght@400;GRAD@0;opsz@24&icon.query=remove&icon.size=24&icon.color=%235f6368

ま、まあ何か参考になればということで。。

@takusea
Copy link
Contributor Author

takusea commented Nov 18, 2024

挙げられている2つのアイコン、良さそうに思いました…！variable_removeはライブラリ側に無かったのでbackspaceを使うのが最適かなと！

あるいはショートカットキーをクリックした後に未割り当てとキーを削除ボタンを用意するとか･･･？

なるほどです。一覧の方に表示するほど重要度が高くないなら従来通りのダイアログの方にいれる形でもいいのかも。ただ、ダイアログが表示された後のキー入力モード内はなるべく出来ることを減らして単純になるようにしたくもあるような…とも思います！

@Hiroshiba
Copy link
Member

Hiroshiba commented Nov 18, 2024 •
edited
Loading

ダイアログが表示された後のキー入力モード内はなるべく出来ることを減らして単純になるようにしたくもある

あ～～～たしかにですね･････。

これ整理すると、情報の構造的に

項目　→　ショートカットキー　→　変更する・削除する・リセットする

なはずなのですが、一覧化しないといけないから項目とショートカットキーを並列に書く必要があって、「ショートカットキーの操作」の場所がないってことなんですね･･･････。
そう考えると右クリックメニューは銀の弾丸ではなく、操作内容に「ショートカットキーを」という主語をかけるからまだましという感じかなぁ。うーーーーん。

ここで良い解決策を見つけるのがUXの真髄な気がしないでもないですが、まあでもちょっと時間かけすぎるのもあれだし諦めるの良さそう！！
難しい判断ですが、

  • ホバーしたら現れる（キーボード操作しづらいのは諦める）
  • ダイアログ内に操作項目を作る（キーボードショートカット編集ダイアログなのに、それ以外もできてしまうのは諦める）
  • 右クリック可能にする（これは妥当なUIが思いついてない）

のどれかですかねぇ･････。

あーーーー完璧な答えである自信がないから、一旦今までの UX(UI？） のままにするのもありかもです！
「操作感を変更するのは最小限にしたい」という気持ちもあるので、自信がない場合はまた後で変更するかもと考えると、今回は見送ってもいいのかも。
（ちょっとUIデザイン的に難しいかもですが･･･ 😇 ）

@takusea
Copy link
Contributor Author

takusea commented Nov 19, 2024

整理ありがとうございます…！

ホバーしたら現れる（キーボード操作しづらいのは諦める）

これは変更ボタンにフォーカスが当たったときも表示という形にすれば、Tabで順々にフォーカスを勧めていく際には違和感は感じないかも？と思いました。ただShift+Tabで戻った際には見えてなかった要素にフォーカスするので多少の驚きはあるかもですが、来た道を戻っている状況だと思うのでそこまで予期してない挙動なことはないかも。ということでそこまで問題でもないのかも…？

「操作感を変更するのは最小限にしたい」という気持ちもあるので、自信がない場合はまた後で変更するかもと考えると、今回は見送ってもいいのかも。

確かにですね…！その場合は元々がデフォルトに戻すが一覧側で未割り当てがダイアログ側だったので新デザイン側でその構造にする感じですかね…！

@Hiroshiba
Copy link
Member

これは変更ボタンにフォーカスが当たったときも表示という形にすれば、Tabで順々にフォーカスを勧めていく際には違和感は感じないかも？と思いました

これは僕も同じこと思ってました！
そして逆順にするとやっぱり変だよなーというのも全く同じことを思っていました･･･！！

これはちょっと良いのかどうなのか、アクセシビリティの考え方がわかんないというのが正直なところです･･･。
ただまぁ経験上、違和感があるポイントがあるということは最高ではないんだろうなーという気持ちもあります 😇

その場合は元々がデフォルトに戻すが一覧側で未割り当てがダイアログ側だったので新デザイン側でその構造にする感じですかね…！

ですね、そんな感じだったと思います！

@takusea
Copy link
Contributor Author

takusea commented Nov 20, 2024 •
edited
Loading

OKです！とりあえずこのPR上では元の構造に戻す形でいこうと思います…！

そういえば、元のデザインだとそもそもデフォルトに戻すボタンにキーボード操作でフォーカスできない（ホバーして表示されている時だけは到達できる）のですが、そこはどうしましょう…！従来の仕様に戻すということには反しますが、到達できないよりは違和感があっても到達できたほうがよさそう…？

Hiroshiba
Hiroshiba reviewed Dec 1, 2024
View reviewed changes
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

一旦一通り見させていただきました！
（まだsrc/components/Dialog/HotkeySettingDialog.vueは未確認）

あ、コンフリクトしてそうなので解決お願いします 🙇

src/styles/v2/colors.scss Outdated Show resolved Hide resolved
src/components/Base/BaseIconButton.vue
defineProps<{
icon: string;
label: string;
disabled?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Storybookでdisable状態の作っても良いかもですね！

src/components/Base/BaseScrollArea.vue Outdated Show resolved Hide resolved
src/components/Dialog/HotkeySettingDialog.vue Show resolved Hide resolved
@takusea
Copy link
Contributor Author

takusea commented Dec 4, 2024

テーブルよりのデザインへの変更と指摘箇所の修正完了です。コンフリクトしてた箇所も直ってるかと！

@Hiroshiba Hiroshiba mentioned this pull request Dec 5, 2024
Merged
1 task
Hiroshiba
Hiroshiba reviewed Dec 5, 2024
View reviewed changes
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

ほぼLGTMです！！

UX周りとデザインのちょっとしたコメントだけ！

src/components/Dialog/HotkeySettingDialog.vue
Comment on lines +226 to +229
const defaultSettings = ref<HotkeySettingType[]>([]);
void window.backend
.getDefaultHotkeySettings()
.then((settings) => (defaultSettings.value = settings));
Copy link
Member

Choose a reason for hiding this comment

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

（ただの報告コメントです）

これ定数なのでexportされてるdefaultHotkeySettingsを直接持ってきてしまったほうが良さそうですね！
ちょっと別でプルリクエスト出してみたので、このプルリクエストがマージされた後にdraftをあけたいと思います！

...ーンショット.spec.mts-snapshots/components-base-baseiconbutton--disabled-light-storybook-win32.png Outdated Show resolved Hide resolved
src/components/Dialog/HotkeySettingDialog.vue Outdated Show resolved Hide resolved
Hiroshiba
Hiroshiba reviewed Dec 7, 2024
View reviewed changes
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

おそらくスナップショットの更新が必要なので（なぜかテストが落ちてないですが）、一度スナップショットを削除したあと更新させていただきます！

Hiroshiba
Hiroshiba approved these changes Dec 7, 2024
View reviewed changes
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM！！

@Hiroshiba Hiroshiba merged commit 0735fbe into VOICEVOX:main Dec 7, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hiroshiba Hiroshiba Hiroshiba approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@takusea @Hiroshiba