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

ソング: 正式なカラーとスタイルを定義する #2218

Open
wants to merge 81 commits into
base: main
Choose a base branch
from

Conversation

romot-co
Copy link
Contributor

@romot-co romot-co commented Aug 13, 2024

内容

ソング機能において正式なカラーを定義し、スタイルを調整することでユーザーの使い勝手を向上させます。
一定程度のアクセシビリティ対応を含みます。
また、今後の拡張を容易にします。

色関連の修正

  • ソングのみ
  • OKLCHベースで色を定義
  • ほぼMaterialDesign3準拠

※ 動的なテーマ生成+テーマエディタ機能のサブセットとなります

表示上の調整

Toolbar

  • 各要素のサイズを小さく・表示修正
  • 拍表示のグループ化
  • シンガーの音域調整・音量調整であることをわかりやすくする

※ 今後Quasarが必須でなくなるのであればradix-vueに変更したい

ScoreSequencer/SequencerGrid/SequencerKeys/SequencerRuler:

  • 不要なグリッド線を表示しない: 不要な重なりによるズレや滲みの除去
  • グリッド線をlineで表示: 目的に応じた塗り分け+(一応)拍子変更に対応できる形にする

SequencerNote

  • 選択中・変更中の表示を明確にする
  • 歌詞表示をズーム倍率に応じて適切なサイズにする
  • 袋文字の色修正(ノートバーからはみだしても見えるように)
  • 歌詞とバーを分割(ノートが狭い感覚で並んだ場合に次のノートの上に文字を表示・ピッチ表示のZ軸揃え)

LyricInput

  • 表示調整
  • 背景を微妙に半透明に: 短いノートが連続している場合に複数入力時に変更があることを一応見えるように

SequencerPitch

  • カラーの調整
  • 基準ピッチを表示する(単にノート縦中央ライン)
    pros: ガイド線となり無いよりは描きやすい・基準ピッチからのずれを把握しやすい
    cons: ガイド線に引っ張られた描き方となる
  • ダーク/ライトテーマで切り替え

関連 Issue

ref #1810
close #1810

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

ノート編集

スクリーンショット 2024-08-28 5 34 16 スクリーンショット 2024-08-28 5 34 37

ピッチ編集

スクリーンショット 2024-08-28 5 33 21 スクリーンショット 2024-08-28 5 33 05

その他

スタイルにおいての変数の使用や書き方については、別途Issue化+すりあわせができればと思います。


設計についての考え方

  1. 各ロールごとの彩度・色相を定義
  2. UIパーツをロールごとに役割をわける
  3. ダーク・ライトごとにロールから明度のみを修正した値を割り当てる

@romot-co romot-co requested a review from a team as a code owner August 13, 2024 20:26
@romot-co romot-co requested review from Hiroshiba and removed request for a team August 13, 2024 20:26
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.

実装周りの詳細な点を除きつつ、一旦全体的な点に関してコメントしました!


デザインすごくいいと思います!!!
ピアノロールの部分かなり違和感なくて驚きました!!

デザインに関して何点かだけちょっとだけ気になったとこがあったのでコメントさせていただきます!
(変えて欲しいというより、意図があればお聞きしたいみたいな感じです!)

スライダーの色が結構黒くて浮いてるかも・・・?
image
image

ノート端ホバー時の太くなり方がちょっと大げさかも?
(これはどっちかというとノート内だけに線が太くなってるから違和感があるだけかも。だとしたら実装的に変更が難しそうなので、今の感じでも・・・!)
image


今後Quasarが必須でなくなるのであればradix-vueに変更したい

必須ではなくなっていく予定です・・・!!

Comment on lines 7 to 23
--font-regular: 400;
--font-medium: 500;
--font-bold: 700;

--input-m-size: calc(var(--size-basis) * 5);
--input-l-size: calc(var(--size-basis) * 7);

--label-l-size: calc(var(--size-basis) * 1.75);
--label-m-size: calc(var(--size-basis) * 1.5);
--label-s-size: calc(var(--size-basis) * 1.25);

--border-width-basis: 1px;

--opacity-medium: 0.87;
--opacity-low: 0.6;
--opacity-separator: 0.5;
--opacity-disabled: 0.38;
Copy link
Member

Choose a reason for hiding this comment

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

この辺りもしかして消し忘れでしょうか 👀

Copy link
Contributor Author

@romot-co romot-co Aug 16, 2024

Choose a reason for hiding this comment

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

@Hiroshiba
こちら消し忘れです!

いちおうtakuseaさんのv2とあわせようと思って修正しようとしたのですが
(追加したものというよりは、すでにあるgapやpaddingやradiusを使っていたもののそちらは削除済み

たぶんどう使うか・変更するか擦り合わせてからのがいいかなーと思っております!

Copy link
Member

Choose a reason for hiding this comment

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

おお、なるほどです!
たしかに合わせていけると良さそう。

Comment on lines 377 to 383
/*
フォールバック
*/

@supports not (color: oklch(0% 0 0)) {
:root[is-dark-theme="false"] {
--scheme-color-primary: #29683a;
Copy link
Member

Choose a reason for hiding this comment

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

ここのフォールバックなるほどです!!

こちらに関して、メンテナンス面考えるとどういう風に扱っていくのがいいでしょう? 👀
上に値を足したり値を変更したら、こっちも更新する感じとか?
(結構大変そう&忘れそうだな~~~~と。。)

候補考えてみました!

  1. なるべく書くということにして、忘れてもまあ別にいいことにする
  2. 書き忘れていないかチェックするテストを書く
  3. 自動更新するスクリプトを用意する
  4. scssを頑張って自動でoklabとrgbどちらも出力するようにする
  5. 思い切って無しにする

個人的には2+3か、あるいは5も結構ありなのかなと思ってます!
5にしておいて、将来未サポートなブラウザもサポートする際にフォールバックと2+3を用意するとか・・・!
(変換スクリプトをすでにお持ちでしたら、そのスクリプトを掲載する・あるいはbuildディレクトリに放り込むとかも現実的かも?)

といっても意外とoklchは全ブラウザの最新版ではれサポートされてそう?
oklabの相対色指定は現状safariでサポートされてないものもありそう。
https://developer.mozilla.org/ja/docs/Web/CSS/color_value/oklch#%E3%83%96%E3%83%A9%E3%82%A6%E3%82%B6%E3%83%BC%E3%81%AE%E4%BA%92%E6%8F%9B%E6%80%A7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Hiroshiba
こちら元々がOKLCHおよびフォールバックの両方をスクリプトで出力しているものなため、
3.がいいのかな…?と思っております。

↓なんか以下のあたり
https://github.com/VOICEVOX/voicevox/pull/2181/files#diff-53a6db5ba78e04948402cbc02136756f43540c63b5b2966278324180cd09c7be

4.ができるかもあわせて検討試行してみます!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SASSでOKLCH→その他の形式に変換はむずかしそう

今後、動的テーマ・カスタムテーマに対応するなら同時にHEX出力すれば大丈夫だが
それまでは3.をがんばるか5.にするか

Copy link
Member

Choose a reason for hiding this comment

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

たしかに動的テーマに対応する場合、必ずjs入るからjsで解決できますね!!

こちら元々がOKLCHおよびフォールバックの両方をスクリプトで出力しているものなため、
3.がいいのかな…?と思っております。

おお、なるほどです!!
そちらがあるならたしかに3も近そうですね!

ただ、あとあと動的テーマのときに解決されるのであればパスしても(=5でも)良いかも・・・・・・?

判断むずいですね!!
個人的には特に他の理由がなければ5寄りの意見です!
(変更用コードはNodeJSで動くようにしないとで結構大変な気がしたので)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Hiroshiba
ありがとうございます!
簡単な手段も見当たらないため、こちらいったん5にします…!

src/styles/v2/sing-colors.scss Show resolved Hide resolved
src/styles/v2/sing-colors.scss Outdated Show resolved Hide resolved
src/styles/v2/sing-colors.scss Outdated Show resolved Hide resolved
src/styles/fonts.scss Outdated Show resolved Hide resolved
src/styles/_index.scss Outdated Show resolved Hide resolved
@romot-co
Copy link
Contributor Author

@Hiroshiba
ありがとうございます!

デザインに関して何点かだけちょっとだけ気になったとこがあったのでコメントさせていただきます! (変えて欲しいというより、意図があればお聞きしたいみたいな感じです!)

スライダーの色が結構黒くて浮いてるかも・・・? image image

こちらすごく要素増えそう…というのがあり、
プライマリは本当に主要機能部分のみ・それ以外の操作できる部分は目立たないセカンダリがいいかな…?
といった意図です。

黒っぽいのはやめておいたほうがいいかもなので、検討します…!

ノート端ホバー時の太くなり方がちょっと大げさかも? (これはどっちかというとノート内だけに線が太くなってるから違和感があるだけかも。だとしたら実装的に変更が難しそうなので、今の感じでも・・・!) image

こちら、ホバー領域が細くてつかみづらいというのがあったため
幅広げた+リサイズハンドルの領域明示化した影響かと思います…!

対応策としては、見かけは細いままで、実質の幅を広げるなど…?

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.

マルチトラック時、他のトラックのノートが置かれてる目印に関して、ライトモードの方の視認性が若干足りてないかも・・・?

ライトモード

image

ダークモード

image

@romot-co romot-co force-pushed the feature/1810_sing_colors_and_styles branch from d76bff1 to 0af8e2f Compare August 17, 2024 02:13
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です!!!

ちょっと色々だいぶ細かいコメントをさせていただきました!!
(今コメントでお伝えすれば、きっと今後ずっとお互いやりやすくなるだろうと思い・・・!)

色定義のSCSSファイルで詳細なドキュメントを書いてくださってありがとうございます!!
どうにかしてオープンソースとしてメンテナンスしやすい色定義の形を模索したいですね・・・!!!

romot-co and others added 5 commits September 17, 2024 22:47
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
@romot-co
Copy link
Contributor Author

前々から出てたのかちょっとわからないのですが、npm run electronしたときにコンソールにSCSS周りでワーニングが出てるようでした!

Warning: 3 repetitive deprecation warnings omitted.

Deprecation Warning: Sass's behavior for declarations that appear after nested
rules will be changing to match the behavior specified by CSS in an upcoming
version. To keep the existing behavior, move the declaration above the nested
rule. To opt into the new behavior, wrap the declaration in `& {}`.

More info: https://sass-lang.com/d/mixed-decls


23  │ ┌   &:last-child {
24  │ │     margin-bottom: 0.6rem;
25  │ │   }
    │ └─── nested rule
... │
27  │     gap: 0px 1rem;
    │     ^^^^^^^^^^^^^ declaration

    src\components\Talk\AudioCell.vue 27:3  root stylesheet

Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($character-item-size, 2) or calc($character-item-size / 2)



60 │           $icon-size: $character-item-size / 2;
   │                       ^^^^^^^^^^^^^^^^^^^^^^^^

    src\components\Dialog\DefaultStyleListDialog.vue 60:23  root stylesheet

[@vue/compiler-sfc] :deep usage as a combinator has been deprecated. Use :deep(<inner-selector>) instead of :deep <inner-selector>.     

Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div(vars.$character-item-size, 2) or calc(vars.$character-item-size / 2)

More info and automated migrator: https://sass-lang.com/d/slash-div


27 │       $icon-size: vars.$character-item-size / 2;
   │                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    src\components\Dialog\CharacterTryListenCard.vue 27:19  root stylesheet

Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($style-item-size, 2) or calc($style-item-size / 2)

More info and automated migrator: https://sass-lang.com/d/slash-div


67 │             $icon-size: $style-item-size / 2;
   │                         ^^^^^^^^^^^^^^^^^^^^

    src\components\Dialog\DefaultStyleSelectDialog.vue 67:25  root stylesheet

Deprecation Warning: Sass's behavior for declarations that appear after nested
rules will be changing to match the behavior specified by CSS in an upcoming
version. To keep the existing behavior, move the declaration above the nested
rule. To opt into the new behavior, wrap the declaration in `& {}`.

More info: https://sass-lang.com/d/mixed-decls

   ╷
54 │ ┌   &:not(.is-in-selected-track) {
55 │ │     border: 1px solid tint(rgba(colors.$primary-rgb, 0.7));
56 │ │     background-image: linear-gradient(
57 │ │       -45deg,
58 │ │       tint(colors.$primary),
59 │ │       tint(colors.$primary) 25%,
60 │ │       tint(rgba(colors.$primary-rgb, 0.36)) 25%,
61 │ │       tint(rgba(colors.$primary-rgb, 0.36)) 50%,
62 │ │       tint(colors.$primary) 50%,
63 │ │       tint(colors.$primary) 75%,
64 │ │       tint(rgba(colors.$primary-rgb, 0.36)) 75%,
65 │ │       tint(rgba(colors.$primary-rgb, 0.36)) 100%
66 │ │     );
67 │ │   }
   │ └─── nested rule
68 │     animation: stripes-animation 0.7s linear infinite;
   │     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ declaration
   ╵
    src\components\Sing\SequencerPhraseIndicator.vue 68:3  root stylesheet

もし直せそうなら・・・・・! 🙇 (大変そうなら一旦そのままでも!)

こちらSASSで関数利用するための
SASSバージョンのアップデートのためかと思います

修正いたしました!

(Quasarもdeprecatedが出ているのですが両方バージョンアップは影響範囲広そうなのいったんこのままで…

@romot-co
Copy link
Contributor Author

@Hiroshiba
(おてすきで)
こちら各所修正済みです

ちょっと色々だいぶ細かいコメントをさせていただきました!!
(今コメントでお伝えすれば、きっと今後ずっとお互いやりやすくなるだろうと思い・・・!)

こちらありがとうございます…!
大きくしすぎた&長くなりすぎたのもあり、だいぶお手数おかけし失礼いたしました…
今後は細かく改善していければと思います


色定義のSCSSファイルで詳細なドキュメントを書いてくださってありがとうございます!!
どうにかしてオープンソースとしてメンテナンスしやすい色定義の形を模索したいですね・・・!!!

メンテしづらい部分があるので
こちらも順次改善していければと思います…!
(動的テーマが入ればむしろ楽になるかも)


またScoreSequencerおよびSequencerNoteの方
以下の形で修正いたしました

  • プレビューモードの一部をcomposableで管理: usePreviewMode(分割や変更を行いやすく)
  • ScoreSequencer: プレビュー状態のスタイルを持たせる(各状態時に両方で制御したいため…)
  • SequencerNote: z-indexの問題で同一コンポーネントにするのが難しそうなため、2つの要素を維持
  • SequencerNote: スタイルの状態を以前よりは明確にする

今よりは修正しやすくしたつもりではあるのですが
もしレビューの手間かかるなど戻す必要があればおしらせください…!

Comment on lines 20 to 29
'rect-selecting': editTarget === 'NOTE' && shiftKey,
previewing: nowPreviewing,
'add-note': previewMode === 'ADD_NOTE',
'resize-note-right': previewMode === 'RESIZE_NOTE_RIGHT',
'resize-note-left': previewMode === 'RESIZE_NOTE_LEFT',
'move-note': previewMode === 'MOVE_NOTE',
'draw-pitch':
(editTarget === 'PITCH' && !ctrlKey) || previewMode === 'DRAW_PITCH',
'erase-pitch':
(editTarget === 'PITCH' && ctrlKey) || previewMode === 'ERASE_PITCH',
Copy link
Member

@Hiroshiba Hiroshiba Sep 19, 2024

Choose a reason for hiding this comment

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

おーーーーなるほどです!!!
ScoreSequencer.vue側にマウスカーソルの形変更を入れるんですね!!
確かに思いつかなかった、こっちのがいいですね!!

これによってSequencerNote側にある、条件ごとのカーソル変更が不要になったかも・・・・・?
例えば↓とかの.movingクラス内のcursor: move;とか。
https://github.com/romot-co/voicevox/blob/8e04c7db683b4b96b81ac7d1641d0854268a53cc/src/components/Sing/SequencerNote.vue#L383-L390

でもそのままでもいいといえば、そのままでも良いかもですねぇ。
カーソルだけはScoreSequencer.vueのみにする・・・・・?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちらScoreSequencer側修正しました

Comment on lines 4 to 6
const previewMode = ref<PreviewMode>("IDLE");
const nowPreviewing = computed(() => previewMode.value !== "IDLE");
const executePreviewProcess = ref(false);
Copy link
Member

Choose a reason for hiding this comment

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

!!!!!
Vueってグローバル変数にrefとかcomputedとか定義しても動くんですね!!!!!!

でもコンポーザブルでこれをやって大丈夫なのかわからないので(ドキュメントには書いてなさそう、実装レベル知ってると分かりそう)、ちょっと避けとくのどうでしょう・・・・・・・・?

もし避けとく場合は、SequencerNote側でusePreviewModeを使うのをやめて、previewModenowPreviewingをpropsで渡すように戻すと良さそう・・・!!

ちなみに今回は親子構造だから割と簡単にこうやって引数渡せますが、もっと深いところとか兄弟に渡そうとすると不便なんですよね。
そういう時はProvide / Injectを使ったりするのが若干一般的かもです。
https://ja.vuejs.org/guide/components/provide-inject#prop-drilling
(Vuexはこの仕組みを使ってどこからでも使えるようになってます。・・・・・たしか。)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます!
グローバル変数は避ける&もしやるなら現状だとVuexで管理だと思うので
usePreviewMode削除して(現状だと再利用できなさそう&そのまま残しておくと危なそう)
ScoreSequencerからSequencerNoteに渡す形にいたしました

Provide/Injectの方ありがとうございます!

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です!!!!!!!
長くありがとうございました!!!!!

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Copy link
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

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

大きくは問題ないと思います。

image

トラック一覧とツールバーの境目が消えてるのが少し微妙かも?

Comment on lines +1575 to +1577
left: -0.5px;
width: 1px;
background: var(--scheme-color-inverse-primary);
Copy link
Member

Choose a reason for hiding this comment

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

image

ぼやき:Windowsだとピクセル単位のズレのせいで線に乗らないの若干気持ち悪いですね...(かといってどうしようもなさそうな予感が)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

のちのちですが: Windows環境で調査

@romot-co
Copy link
Contributor Author

@sevenc-nanashi

トラック一覧とツールバーの境目が消えてるのが少し微妙かも?

こちらマージ後、別の形で修正できればと思います!
(サイドバーの調整も合わせ

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.

ソング:シング側の色を揃える・正式な定義
4 participants