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

#1818 ソング:MIDIトラックを選択できる ようにする #1892

Merged
merged 10 commits into from
Mar 20, 2024

Conversation

romot-co
Copy link
Contributor

@romot-co romot-co commented Mar 3, 2024

内容

  • ソングの「MIDIインポート」においてダイアログを表示し、ファイルとともにトラックを選択できるようにします

複数トラックのMIDIの場合、目的のトラックがインポートできない問題を解決します

関連 Issue

ref #1818
close #1818

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

スクリーンショット 2024-03-03 9 12 26

その他

  • MusicXML側は修正していません
  • IMPORT_MIDI_FILEとで2回MIDIファイルをパースしてますが、ファイルドロップまわりがどうなるか把握できていないため、いったんそのままです

@romot-co romot-co requested a review from a team as a code owner March 3, 2024 00:16
@romot-co romot-co requested review from Hiroshiba and removed request for a team March 3, 2024 00:16
@romot-co romot-co force-pushed the feature/1818_import_midi_selectable_track branch from 8847bef to 778db35 Compare March 3, 2024 00:18
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です!!

コード読みやすかったです!!

src/store/singing.ts Outdated Show resolved Hide resolved
src/components/Dialog/ImportMidiDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportMidiDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportMidiDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportMidiDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportMidiDialog.vue Outdated Show resolved Hide resolved
@romot-co
Copy link
Contributor Author

romot-co commented Mar 6, 2024

@Hiroshiba
お忙しいところレビューいただいて本当に助かります!
各点、ほかIssueもあわせてのちほど修正いたします!

@romot-co
Copy link
Contributor Author

romot-co commented Mar 6, 2024

@Hiroshiba
こちらご指摘ありがとうございます!取り急ぎ各点修正いたしました。

ファイルドロップもやりかけたのですが、
ドロップ時にトラック選択する必要あるため、Dialog表示になる...
Dialogまわり把握できておらず&おそらくはドロップの場合はDialogでファイルパス名のみ表示などになる&storeにドロップしたファイルパス持たせるとクリーンナップがややこしそう…などで今後の課題にできれば…!

@romot-co romot-co requested a review from Hiroshiba March 6, 2024 15:29
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.

ちょっと1個バグ見つけたのでコメントしました!

デザインに関して、ちょっとこちらで他のダイアログと揃うように後々調整させていただくかもです!

src/components/Dialog/ImportMidiDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/AllDialog.vue Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
romot-co and others added 4 commits March 19, 2024 06:51
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
…ble_track' into feature/1818_import_midi_selectable_track
@romot-co
Copy link
Contributor Author

指摘点修正済みです
(今投げてしまっているプルリク以降、forkしたリポジトリからgit操作するようにいたします…!

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

ファイルのドラッグアンドドロップに関して、ちょっと想像されてるのと違うかもですがご案内です!!

ダイアログは、SingEditor内にドロップされたパスを、引数でダイアログに渡せるようにするのが綺麗に感じます。
実はAllDialog内に記述しなくても、Dialog.createという関数を使えばprops付きでquasarダイアログを開けたりします。こっちも参考になれば・・・!!

Dialog.create({
component: SaveAllResultDialog,
componentProps: {
successArray: successArray,
writeErrorArray: writeErrorArray,
engineErrorArray: engineErrorArray,
},
});

プロジェクトファイルに関して、トーク側にこそっとファイルD&Dが実装されていたりするので参考になれば!!

const loadDraggedFile = (event: { dataTransfer: DataTransfer | null }) => {

@Hiroshiba Hiroshiba merged commit 10e5b64 into main Mar 20, 2024
18 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.

ソング:MIDIインポートでトラックを選択できるようにする
2 participants