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

設定ダイアログのリデザイン #2276

Merged
merged 44 commits into from
Oct 12, 2024

Conversation

takusea
Copy link
Contributor

@takusea takusea commented Sep 29, 2024

内容

設定ダイアログのリデザインを行います。コンポーネントごとの変更内容は以下のとおりです。

BaseToggleGroup・BaseToggleGroupItem

  • 記述の整理
  • transition関連のスタイルの調整
  • Tooltipと併存できるようにCSSセレクタを変更

BaseRowCard

  • 幅が狭いときに崩れないよう折り返し方を修正

BaseSelect・BaseSelectItem

  • コンポーネントを追加

BaseCheckbox

  • コンポーネントを追加
  • Storyファイルを追加

ButtonToggleCell・EditButtonCell・ToggleCell

  • BaseRowCardを使った実装に変更

BaseCell

  • BaseRowCardとほぼ同一になったため削除

SettingDialog

  • 親コンポーネントに置き換え
  • 「書き出し先を固定」と「書き出し先のフォルダ」をのcellを分離
    • Before image
    • After image

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

image

@takusea takusea requested a review from a team as a code owner September 29, 2024 17:33
@takusea takusea requested review from Hiroshiba and removed request for a team September 29, 2024 17:33
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.

全体的に柔らかい感じになっていて、良い感じだと思いました!!

ちょっとまだコードが追えてないのですが、実際に表示していくつか気になった点をコメントしました!

(気になる点を分けてディスカッションしやすいよう、適当なファイルにコメントさせていただきました 🙇 )

:options="availableAudioOutputDevices"
class="col-7"
</ToggleCell>
<QSlideTransition>
Copy link
Member

Choose a reason for hiding this comment

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

(ただのコメントです)

あ、ここquasar成分残ってそうですね!
デザインの方針の参考になればと思ってちょっとコメントまで。

ぶっちゃけアニメーションするのはこだわりなくて、ONのときに現れたということが分かりやすいようにしたかった感じです。
クリックする対象と現れる対象を見た目的に親子構造にすると良さそうなのですが、たしか離れてるところに現れるやつもあるので親子構造とかには今はできなそう。。。 😇

@takusea
Copy link
Contributor Author

takusea commented Oct 4, 2024

SelectCellの作成&RowCard全体を押せるようにするのと、項目がdisabledだったときの動作を修正しました。

@takusea
Copy link
Contributor Author

takusea commented Oct 8, 2024

ホバー・クリック時の色を変更するアニメーションを削除してみました。
レスポンス感が向上してサクサクしているような感じはしますが、ツール的な堅さが感じられると言われたらそのような気も。

@sevenc-nanashi
Copy link
Member

個人的な好みだけで言うと、アニメーションがあるとテンションが上がって作業が進むのでアニメーションは必ずしも消さなくてもいいと思います。

@Hiroshiba
Copy link
Member

個人的には少なくとも問題にはならなさそうには感じました!!

もしかしたらSelectが開かなくなってるかも・・・?(勘違いだったらすみません 🙇 )

@takusea
Copy link
Contributor Author

takusea commented Oct 9, 2024

もしかしたらSelectが開かなくなってるかも・・・?(勘違いだったらすみません 🙇 )

こっちだと普通に開けてそうです!

@@ -0,0 +1,114 @@
<template>
<SelectRoot v-model="model" v-model:open="open" :defaultValue :disabled>
Copy link
Member

Choose a reason for hiding this comment

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

めちゃめちゃ細かいのですが不具合っぽい挙動を発見しました!!
でもなんかradix-vue側の課題な気もしますが。

Selectを開く→画面内の適当なとこをクリックしてSelectを閉じる→もう一度Selectを開く→マウスクリックを離したタイミングで閉じてしまう

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.

こっちだと普通に開けてそうです!

すみませんでした、開けました!!!


長いレビューに付き合ってくださってありがとうございます!!!
あとはSelectArrowの部分でワーニングが出てる件だけ変更して完成かなと思います!!!
デザイン的にもコード的にも良い感じになったと感じます!!

不要なクラスの削除もありがとうございます!!

あ、色のアニメーションですが、テーマ変更時に妙なことにならなければ設定されていても全く問題ないと思います・・・!
(アニメーションをなくす方に倒してほしい、という押し付けになってしまっていたら申し訳ないなと思ってのコメントです 🙇 )

@takusea
Copy link
Contributor Author

takusea commented Oct 11, 2024

全然押し付けとは感じてなかったです…!ホバー時などの色の変化へのアニメーションは、あるにしても柔らかくモダンな印象が得られてモチベになったり、ないにしてもサクサク感があり制作作業に快適性が得られるというように両方にメリットがあると考えていて、後はVOICEVOX的にどちらに倒すかだけだと思ってます。
ただ、VOICEVOXはどっちも重要視してそうなので、どちらに倒そうとも個人的に異論はないと思ってます。

まあでも、もう少しアニメーションの時間を短くしたり、イージングの仕方を変えるなどが最適な案だという結論に最終的に至るかもしれないので、変更を急いでこのPRにひとまとめにせずいずれ検討する形でもいいのかもですね…!

@Hiroshiba
Copy link
Member

失礼しました、ありがとうございます 🙇 🙇 🙇

まあでも、もう少しアニメーションの時間を短くしたり、イージングの仕方を変えるなどが最適な案だという結論に最終的に至るかもしれないので、変更を急いでこのPRにひとまとめにせずいずれ検討する形でもいいのかもですね…!

確かにです!
この辺の時間方向の UX はクリエイター向けソフトならではなので、知見をどんどん貯めていきたいですね・・・!!!

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!!!

調整ありがとうございました!!!!
コンフリクト発生しているので、こちらで調整したのをpushさせていただきます!!

こんな感じでツイートさせていただこうと思います!

#VOICEVOX開発状況 
設定ダイアログのデザインが変更され、見やすくなりました🎉(今後のアップデートで実装されます。)
【開発者:@takusea】
https://github.com/VOICEVOX/voicevox/pull/2276
(画像添付)

image

@Hiroshiba
Copy link
Member

あっテスト落ちてますね・・・!
これ結構playwrightの知見が必要なので、僕側で治すの挑戦してみます!

@takusea
Copy link
Contributor Author

takusea commented Oct 12, 2024

あっテスト落ちてますね・・・! これ結構playwrightの知見が必要なので、僕側で治すの挑戦してみます!

ありがとうございます!お願いします…!

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.

ちょっと手間取ってコミット数が増えてしまってすみません!
マージします!!

(以下ちょっとメモ)

設定ダイアログはスクリーンショットテストをしてるんですが、マウスホバーが入って入ってないのがあるかも。
77898f1 (#2276)
ホバーが入ってない方はセルにマウスカーソルが当たってないだけなのか、はたまたホバー判定が入る前にスクリーンショットが取られたのかどうか不明
もし後者だったらスクリーンショットが撮られるタイミングによって画像が変わってしまってテストが落ちるかも。
もしスクショテストが落ちたらここを疑う。

@Hiroshiba Hiroshiba merged commit 4dbc9a2 into VOICEVOX:main Oct 12, 2024
8 checks passed
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.

3 participants