project-s: UIスケルトンの用意 #977
Conversation
There was a problem hiding this comment.
LGTM！！
全然関係ないのですが、VOICEVOXを開発したての頃はこんな感じのプロトタイプ感あるUIだったので、すごく懐かしくなりました。
ちなみに、実はVueはcomponentsディレクトリのサブディレクトリを推奨していなかったりします。
https://v3.ja.vuejs.org/style-guide/#%E5%AF%86%E7%B5%90%E5%90%88%E3%82%B3%E3%83%B3%E3%83%9B%E3%82%9A%E3%83%BC%E3%83%8D%E3%83%B3%E3%83%88%E3%81%AE%E5%90%8D%E5%89%8D-%E5%BC%B7%E3%81%8F%E6%8E%A8%E5%A5%A8
が、project-sの開発段階ではあまり気にせず、隔離しやすいという意図でこの形式もありだと思いました！
src/components/Sing/MenuBar.vue
Outdated
| </template> | ||
|
|
||
| <script lang="ts"> | ||
| import { defineComponent, ref, computed, ComputedRef, watch } from "vue"; |
There was a problem hiding this comment.
Github上でFiles changedを見ると'watch' is defined but never usedみたいなワーニングがいっぱい出てました。
別にそのままでも良いのですが、レビューするときにちょっと見づらかったのでコメントしました･･･！
あまりに利用していない場合はコメントアウトしても良いかも･･･？
There was a problem hiding this comment.
@Hiroshiba
ありがとうございます、わかりづらく失礼しました！
おそらく使うだろうものを残している意図ですが、
筋もよくないと思うので各所削除し、利用時追加にいたします！
src/main.ts
Outdated
| @@ -24,7 +24,7 @@ createApp(App) | |||
| .use( | |||
| createGtm({ | |||
| id: process.env.VUE_APP_GTM_CONTAINER_ID ?? "GTM-DUMMY", | |||
| vueRouter: router, | |||
| //vueRouter: router, | |||
There was a problem hiding this comment.
@Hiroshiba
こちら特に理由ないため修正します！
| // NOTE: UIの状態などは分割・統合した方がよさそうだが、ボイス側と混在させないためいったん局所化する | ||
| isShowSinger: boolean; | ||
| // NOTE: オーディオ再生はボイスと同様もしくは拡張して使う？ |
There was a problem hiding this comment.
UIの状態管理を混ぜるべきか分けるべきか、迷いますよね･･･。
いったんとりあえず全部分けちゃって、あとで混ぜるか考える、とかで良いかもと思いました！
There was a problem hiding this comment.
@Hiroshiba
ありがとうございます、
こちら本来はUiStoreStateに入るものかなあ…と思うのですが、
いったんSingingStoreStateにまとめてしまって後で分割できれば…と考えています！
問題あればおしらせください！
|
@Hiroshiba @sigprogramming
特に問題ないかと思いますが、必要そうでしたらおてすきでご確認いただければ幸いです。 ディレクトリはどうしよう…なのですが、試行のためいったんそのままにしております。 UIは現状ほんとにスケルトンなので、表示は実装あわせ修正していければと思います！ |
|
全体的に良いと思いました！！ |
|
@Hiroshiba |
内容
project-s:
/components/Sing以下関連 Issue
※ 別にIssue作成したもののわかりづらいため、次回以降voicevox側に作成いたします
ref: VOICEVOX/voicevox_project#15
ref: VOICEVOX/voicevox_project#16
スクリーンショット・動画など
その他