Add: シーケンサにおいてツール選択可能にする #2367
Add: シーケンサにおいてツール選択可能にする #2367
Conversation
…r_click_selectable_various_mode_mock
…r_selectable_edit_mode
…r_selectable_edit_mode
…/github.com/romot-co/voicevox into feature/2039_sequencer_selectable_edit_mode
…/github.com/romot-co/voicevox into feature/2039_sequencer_selectable_edit_mode
…r_selectable_edit_mode
|
🚀 プレビュー用ページを作成しました 🚀 更新時点でのコミットハッシュ： |
There was a problem hiding this comment.
Copilot reviewed 6 out of 11 changed files in this pull request and generated no suggestions.
Files not reviewed (5)
- src/styles/_index.scss: Language not supported
- src/styles/fonts.scss: Language not supported
- src/components/Sing/ScoreSequencer.vue: Evaluated as low risk
- src/components/Sing/SequencerNote.vue: Evaluated as low risk
- src/components/Sing/SequencerToolPalette.vue: Evaluated as low risk
…r_selectable_edit_mode
There was a problem hiding this comment.
Copilot reviewed 7 out of 11 changed files in this pull request and generated no suggestions.
Files not reviewed (4)
- src/styles/_index.scss: Language not supported
- src/styles/fonts.scss: Language not supported
- src/components/Sing/SequencerNote.vue: Evaluated as low risk
- src/components/Sing/SequencerToolPalette.vue: Evaluated as low risk
Comments skipped due to low confidence (1)
src/components/Sing/ToolBar/EditTargetSwicher.vue:35
- There is a misspelling in the comment. 'transitionSide' should be 'transitionHide'.
transitionSide=""
There was a problem hiding this comment.
ほぼLGTMです！！
色々コメント書いていますが、とんちんかんなことも言ってるかもなので遠慮なくご指摘いただければと思います 🙇
挙動の細かい制御のとこ、コメント書きまくるの良いですね！！
コメントでロジックのチェックをした後、コメントと共同が揃ってるかどうか確かめる手が使えそう。
z.enumで定義してそれをzodSchemaとして使わないのはちょっとYAGNI感があるので、普通のtypeのが良いかもです！ 🙇
これに編集状態を保存する場合はSchemaが必要になると思うので、その時に実装する感じが良いかなと！
（たぶんtype/preload.ts辺りに移動も必要）
こう書き換えられるはず。
// const hogeSchema = z.enum(["A", "B"])
// type hoge = typeof()
// ↓
type hoge = "A" | "B"まあでも全く問題ではないので、もしよかったらくらいで･･･！
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
…/github.com/romot-co/voicevox into feature/2039_sequencer_selectable_edit_mode
There was a problem hiding this comment.
Copilot reviewed 7 out of 11 changed files in this pull request and generated no suggestions.
Files not reviewed (4)
- src/styles/_index.scss: Language not supported
- src/styles/fonts.scss: Language not supported
- src/components/Sing/SequencerNote.vue: Evaluated as low risk
- src/components/Sing/SequencerToolPalette.vue: Evaluated as low risk
| // Ctrlキーが押されたときにピッチツールを変更したかどうか | ||
| const toolChangedByCtrl = ref(false); | ||
|
|
||
| // ピッチ編集モードにおいてCtrlキーが押されたときにピッチツールを消しゴムツールにする | ||
| watch([ctrlKey], () => { |
There was a problem hiding this comment.
（細かいですが）
watchはビフォーアフターの値をどちらも受け取れるので、false→trueになったときの処理と、true→falseになった時のifにすればtoolChangedByCtrl状態変数を消せるかも？
あとすごい細かいですが、ctrlを押しながらeditTargetを変更した場合にERASEにならないかもですね！
まあこっちもdetermineかcomputedにしたほうが良いかも。
（でもかなり細かいので今はどちらでも良さそう）
There was a problem hiding this comment.
こちらおっしゃるとおり＆その形を考えたのですが
現状での期待動作が以下のため状態変数が必要かなーという形です
DRAW選択時 → Ctrlキー押す → → Ctrlキー離す → DRAWに戻る
ERASE選択時 → Ctrlキー押す → なにも起こらない(DRAWに変更されない)
単純にCtrlキーやPitchToolの新旧比較ではCtrlキー離されたときに常にツールがDRAWに戻ってしまうため
一時的な切り替えであることを保持しておく必要がある気がする
determineにまとめようとしたものの単にいい方法が思いつかなかった感じなので
コメントで意図の付与のみ行いました…！
There was a problem hiding this comment.
あっ！！確かにです！！！
ちなみにdetermineでやる場合は、ツールの選択状態変数やctrlKeyから、「どのツールを使うか」を計算する感じになるかなと思ってます！
今多分VuexのsequencerPitchTool状態変数をそのまま使ってると思うのですが、これを
// どのツールを使うかの計算結果が入る
const nowPitchTool = computed(() => {
// ctrlKeyが押されていたら問答無用で ERASE 、そうじゃなければ状態変数のものを使う
return ctrlKey ? "ERASE" : state.sequencerPitchTool
})とする感じをイメージしてました！
（nowPitchToolという名前はもっと良いのがありそう）
…/github.com/romot-co/voicevox into feature/2039_sequencer_selectable_edit_mode
|
マージします！！！！！！！！！！！！ |
|
15時あたりにこんな感じでツイートさせていただこうと思います！！ 🙏
|
Co-authored-by: Hiroshiba <[email protected]>
内容
以下の機能を追加します
関連 Issue
ref #2039
close #2039
スクリーンショット・動画など
test-tools-scr.mp4
その他
ツールパレットの位置やUI構成については考慮必要かもしれないですが、
まずは機能追加を行いボリューム編集を取り込み後に適宜修正できればと考えています
Material Iconsに適切なものがなかったため
Material Symbolsを取り入れています(別Issueの方がよさそうでしたらおしらせください)
※ 再作成します