[project-s] ドラッグ周りの整理とプレビュー処理の追加 #1680
Conversation
|
レビュー中です！ バグっぽい挙動に気づいたので先にコメントだけ。 ノート追加の判定がノート長の半分左にずれてるかもです。 |
| const selectedNoteIds = new Set(state.selectedNoteIds); | ||
| for (const noteId of noteIds) { | ||
| selectedNoteIds.add(noteId); | ||
| } | ||
| state.selectedNoteIds = selectedNoteIds; |
There was a problem hiding this comment.
もともとselectedNoteIdsはsetなので、こうで良いかと！
| const selectedNoteIds = new Set(state.selectedNoteIds); | |
| for (const noteId of noteIds) { | |
| selectedNoteIds.add(noteId); | |
| } | |
| state.selectedNoteIds = selectedNoteIds; | |
| for (const noteId of noteIds) { | |
| state.selectedNoteIds.add(noteId); | |
| } |
| const selectedNoteIds = new Set(state.selectedNoteIds); | ||
| for (const noteId of noteIds) { | ||
| selectedNoteIds.delete(noteId); | ||
| } | ||
| state.selectedNoteIds = selectedNoteIds; |
There was a problem hiding this comment.
ここもこうできそう？
| const selectedNoteIds = new Set(state.selectedNoteIds); | |
| for (const noteId of noteIds) { | |
| selectedNoteIds.delete(noteId); | |
| } | |
| state.selectedNoteIds = selectedNoteIds; | |
| for (const noteId of noteIds) { | |
| state.selectedNoteIds.delete(noteId); | |
| } |
| const selectedNoteIds = new Set(state.selectedNoteIds); | ||
| for (const noteId of noteIds) { | ||
| selectedNoteIds.add(noteId); | ||
| } | ||
| state.selectedNoteIds = selectedNoteIds; |
There was a problem hiding this comment.
ここも。
| const selectedNoteIds = new Set(state.selectedNoteIds); | |
| for (const noteId of noteIds) { | |
| selectedNoteIds.add(noteId); | |
| } | |
| state.selectedNoteIds = selectedNoteIds; | |
| for (const noteId of noteIds) { | |
| state.selectedNoteIds.add(noteId); | |
| } |
| let draggingNoteId = ""; | ||
| let edited = false; | ||
| // ダブルクリック | ||
| const clickedNoteIds: (string | undefined)[] = [undefined, undefined]; |
There was a problem hiding this comment.
結構大きなコンポーネントなので、ダブルクリック判定用だとわかる変数名にしておくと安心かもと思いました！
あと型をtupleにしておけば要素数が２つだということも明示できます！
| const clickedNoteIds: (string | undefined)[] = [undefined, undefined]; | |
| const clickedNoteIdsForDoubleClick: [string | undefined, string | undefined] = [ | |
| undefined, | |
| undefined, | |
| ]; |
| if (editedNotes.length === 0) { | ||
| return; | ||
| } | ||
| store.dispatch("UPDATE_NOTES", { notes: editedNotes }); |
There was a problem hiding this comment.
UPDATE_NOTESにとっての例外処理だと思うので、UPDATE_NOTES内に書いてあげたほうがシンプルかなと思いました！
| const noteNumber = baseYToNoteNumber(eventOffsetBaseY); | ||
| if (noteNumber < 0) { | ||
| return; | ||
| // プレビュー |
There was a problem hiding this comment.
ちょっとここにFIXMEコメントだけ書かせていただけると･･･！
| // プレビュー | |
| // プレビュー | |
| // FIXME: 関連する値を１つのobjectにまとめる |
| let currentCursorY = 0; | ||
| let dragStartTicks = 0; | ||
| let dragStartNoteNumber = 0; | ||
| let draggingNoteId = ""; |
There was a problem hiding this comment.
やっぱり型で異常かどうか判別できる方が良いかなと思います。
| let draggingNoteId = ""; | |
| let draggingNoteId = ""; // FIXME: 無効状態はstring以外の型にする |
たぶんPreviewModeもundefinedになれるようにして、PreviewModeがundefinedじゃなければdraggingNoteIdも型として現れる形が良いかなと。
こんな感じを想像してます。
const previewValue: {
mode: undefined;
} | {
mode: PreviewMode;
draggingNoteId: string;
// 他変数いろいろ
}| const onMouseMove = (event: MouseEvent) => { | ||
| if (!nowPreviewing.value) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
良い方法か若干自信がないですが、最初からイベントリスナー登録するのではなく、プレビュー開始したらこのイベントをaddする手もあるなと思いました！
| if (previewMode === "RESIZE_LEFT") { | ||
| previewResizeLeft(); | ||
| } | ||
| previewRequestId = requestAnimationFrame(preview); |
There was a problem hiding this comment.
再レンダリングが必要なくてもpreviewが呼ばれ続けるなと思いました！
onMouseMoveしたときにこの関数が呼ばれる形でも良いかも？
|
マージさせていただきます！！！ |
|
レビューありがとうございます！
他のソフトではfloorではなくroundになっていたので、それに合わせました！ |
おっとなるほどです！ VOICEVOX.-.Ver.999.999.999.2023-12-29.13-49-15.mp4 |
内容
以下を行います。
cursor: autoにする（カーソルのちらつきが発生しないように）dblclickイベントをノートではなくsequencerBodyで受け取るようにするkeydownイベントをノートではなくdocumentで受け取るようにするstate.selectedNoteIdsとstate.overlappingNoteIdsの型をSetに変更state.sequencerScrollXとstate.sequencerScrollYを削除関連 Issue
VOICEVOX/voicevox_project#15
その他