Skip to content

Conversation

@romot-co
Copy link
Collaborator

@romot-co romot-co commented Jan 26, 2025

内容

以下の内容を行います

  • ループ範囲をドラッグおよびメニューで追加・削除できるようにする
  • ループをON/OFFできるようにする
  • ループ設定をプロジェクトに保存し復帰できるようにする
  • ミニマップ(トラックリスト)からもループすることを見越す
  • storybookにLoopLaneを追加

関連 Issue

ref #2224
close #2224

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

2025-01-26.20.46.43.mp4
スクリーンショット 2025-01-26 20 50 04 スクリーンショット 2025-01-26 20 49 44

その他

  • マージの解決がややこしかったため、別ブランチ・Issueとして作成いたします
  • Grid/ValueChanges/Loopなどレーンごとにコンポーネントとして分割(...逆に複雑になった気もしますが、コンテキストメニュー制御などがややこしかったため
  • Containerにロジック/Presentationはなるべくステートレスかつ表示のみの責務を目指したつもり

@romot-co romot-co requested a review from a team as a code owner January 26, 2025 12:02
@romot-co romot-co requested review from Hiroshiba and removed request for a team January 26, 2025 12:02
@romot-co romot-co marked this pull request as draft January 26, 2025 12:03
@romot-co romot-co changed the title ループの機能追加 ソング:ループの機能追加 Jan 26, 2025
@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Jan 26, 2025

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

更新時点でのコミットハッシュ:0ff954a

@Hiroshiba
Copy link
Member

コンフリクトの解決とか、不明な点とかあれば何でも聞いてください!!!!

@romot-co romot-co marked this pull request as ready for review January 27, 2025 15:03
@romot-co
Copy link
Collaborator Author

@Hiroshiba
おつかれさまです!(おてすきで)
テストにつきまして、engineMockのあたりがわかっておらず…どのようにすればよさそうでしょうか…!
https://github.com/VOICEVOX/voicevox/actions/runs/12991557907/job/36229342596?pr=2506

@Hiroshiba
Copy link
Member

テストにつきまして、engineMockのあたりがわかっておらず…どのようにすればよさそうでしょうか…!

ちょっと試した感じ、変更前に戻せばよさそうでした!!
勝手ながらコミット追加させていただきます! 🙏

聞いてくださってありがとうございます!!! また何でも聞いてください!!!

@romot-co romot-co requested a review from Copilot January 28, 2025 12:04
@romot-co
Copy link
Collaborator Author

storybookに問題出ているのでそちら修正予定

@romot-co
Copy link
Collaborator Author

romot-co commented Jan 30, 2025

そのままやるとSequencerRulerのContainer責務重すぎ&実装グダグダになったので子にそれぞれミニContainer持たせてロジックわける形にしたものの
親Container+各ロジックはcomposableに切り出す+各子コンポーネントはPresentation相当のみ…もありだったかも

@sevenc-nanashi sevenc-nanashi self-requested a review September 4, 2025 11:31
@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 11, 2025

@sevenc-nanashi 再レビューお願いできると・・・!!

個人的には、雰囲気ちょっと行数がとんでもないことになっているので、コードに大きな問題がなさそうかチェックして、あとは実行して動けば、えいやでマージするのが良い・・・というよりそうするしかないかなと思っています!

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

@Hiroshiba Hiroshiba changed the title ソング:ループの機能追加 feat: ソング:ループの機能追加 Sep 13, 2025
@Hiroshiba
Copy link
Member

@sevenc-nanashi approveって感じですかね?
であればマージ押して頂けると!!(確認の意図だけ)

@sevenc-nanashi
Copy link
Member

ん〜気持ちとしてはまだRequest for Changesの気持ちですね(baselineが揃ってないとかなり不格好に感じてしまうので…)

とは言っても自分の好みだけな気がします。直すのが難しそうなら一旦Approveして別のPRで作業しようかなと

@romot-co
Copy link
Collaborator Author

@sevenc-nanashi @Hiroshiba
見た目や触り心地など正直ちょっと微妙なので、見た目だけ変える範囲で調整できればと思います!

※ 別Issue化することも考えましたが、ループについてはやってしまったほうがよさそう...

@romot-co
Copy link
Collaborator Author

@sevenc-nanashi @Hiroshiba
こちら以下のような形になるよう修正しました。
スクリーンショット 2025-09-20 22 47 39

  • ループレーンを背景色を変えて、他とは別かつ(わずかに)操作可能エリアに見せる
  • 拍数字と被らないようにする
  • 操作有効エリアを縦1/3ぐらいにする / ハンドル部はもっと広く...プレイヘッド位置移動との兼ね合い

ほかにやっておいた方が良さそうなことだけど別Issue想定:

  • 被発見性の向上: ループ操作できそうなエリアかわかるように...まあ、なくてもよさそう
  • ドラッグ時のカーソルの固定: ループだけではないので、ドラッグ時に全画面でオーバーライドするフックがあればよさそう?
  • オートスクロール: これもループに限らず
  • キーボードショートカット
  • シーケンサ側選択範囲などからのループエリア設定: 余裕・必要あれば

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 21, 2025

@romot-co

あ!
1点だけ気づいたのですが、ダブルクリックが削除になってるのがちょっと気になりました!
どうすべきかわからないのですが、ダブルクリックは現状編集寄り(といってもノートダブルクリックくらいかも)なので、どうすべきなんだろうってくらいの気持ちです!
ちなみにピッチをもとに戻す=削除するのはctrl+ドラッグ、テンポマーカーとかの削除は右クリック→メニュー→削除。

あとループ範囲外のループレーンをクリックすると、おそらく範囲0のループを作ろうとして、前のループ範囲を消す挙動になってるのって意図的でしょうか?(確認したい意図です)
シーケンサーを選択優先モードのときに範囲選択したあと選択を解除するのはシングルクリックなので、統一が取れている・・・・・・・?
いやでもループ範囲削除と選択解除は違いそうなので、UX的に理想何を理想状態とするか次第かも。

このPRで全部完璧にするのは時間的に難しいと思うので、今のとこゴールをどこにするのかだけふんわり決まっていれば良いのかなと!

@romot-co
Copy link
Collaborator Author

@Hiroshiba
確認しました!

1点だけ気づいたのですが、ダブルクリックが削除になってるのがちょっと気になりました!

確かに!ダブルクリックでの削除はなくせればと思います!

作業の補助線みたいなものなので、気軽に追加・削除できていいと思うのですが、謎挙動ではあるので...


あとループ範囲外のループレーンをクリックすると、おそらく範囲0のループを作ろうとして、前のループ範囲を消す挙動になってるのって意図的

これは意図的ではないです。

別のループレーン作る場合は前のを消すという挙動にしたかったのですが、
範囲0でもそうなってしまうため、クリックで消えてしまうように見えてしまっているかと思います...
すごくややこしくなければ

  • 新しいループ範囲が閾値を越えたら or 新しいループ範囲が作成されたら前のを消す

という形にできればと思います。

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 21, 2025

新しいループ範囲が閾値を越えたら or 新しいループ範囲が作成されたら前のを消す

なるほどです! 今は理想的じゃないって感じですかね!
とりあえず理想はこう、というのがわかれば良いと思うので、このPRは今の挙動にしておいて、理想をissue作るのもありだと思います。どちらでも!!

@romot-co
Copy link
Collaborator Author

こちら

  • ダブルクリックでループ範囲消去機能を削除
  • LoopLaneクリックで既存ループ範囲が消去に見えないようにする(移動範囲が閾値をこえるまではループ作成を開始しない)

という形で修正しました

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.

触った限りだと問題なさそう。

To: @romot-co
マージして大丈夫ですかね?

@romot-co
Copy link
Collaborator Author

@sevenc-nanashi
レビューありがとうございます、
はい、問題ありません

@sevenc-nanashi
Copy link
Member

sevenc-nanashi commented Sep 27, 2025

マージします!

Twitterで新機能ツイートするレベルの変更っぽそうなのでcc: @Hiroshiba

@sevenc-nanashi sevenc-nanashi added this pull request to the merge queue Sep 27, 2025
Merged via the queue into VOICEVOX:main with commit a31189d Sep 27, 2025
11 checks passed
@Hiroshiba
Copy link
Member

たしかにポストしたいですね!!
こんな感じで明日のAM11:00にポスト予約しておこうと思います!

#VOICEVOX開発状況 
ソングにループ機能が実装されました🎉(今後のアップデートで実装されます。)
【開発者:@romotco】
https://github.com/VOICEVOX/voicevox/pull/2506

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