ダイアログをコンポーネント化 #1
Conversation
| @@ -0,0 +1,31 @@ | |||
| <template> | |||
| <q-dialog v-if="props.isUpdateAvailable" v-model="showUpdateNotificationDialog"> | |||
There was a problem hiding this comment.
v-if と v-modelは呼び出し側で指定した方が良いかも
i.e. (Engine.vue)
<update-notification-dialog
v-if="props.isUpdateAvailable"
v-model="showUpdateNotificationDialog"
/>
|
|
||
| const showUpdateNotificationDialog = ref(true); | ||
|
|
||
| const closeUpdateNotificationDialog = () => { |
There was a problem hiding this comment.
これを EditorHome.vue に移すことになる。
良い点
- 既存の他のコンポーネントと形式を揃えられる
懸念点
- show と close が別の場所にあるのはキモいかもしれない(けど自然にも思える)
| <script setup lang="ts"> | ||
| import { ref } from "vue"; | ||
|
|
||
| const props = |
There was a problem hiding this comment.
ポップアップを表示するかどうかは EditorHome が制御したほうがいいと思ったけど、そうすると props の存在意義がほぼ無くなり、downloadLinkもハードコーディングで良いのでは？という気持ちになる()
There was a problem hiding this comment.
ポップアップの表示/非表示をEditorHomeに任せるのは賛成!
isUpdateAvailableについては
#235 でpropsじゃなくてアップデート確認用の関数(HelpDialog.vueで書かれてる部分を関数としてどこかに定義しておく)を呼び出して受け取るのがいいとのこと.
downloadLinkに関してはハードコーディングでいいと思う.
グローバル変数にするほどのことでもなさそうだし
There was a problem hiding this comment.
isUpdateAvailableは一旦置いといて(EditorHome で isUpdateAvailable = true とかにしといて)、その他の変更を反映させてmergeしてくれる？
isUpdateAvailableはそれから考えよう！
There was a problem hiding this comment.
長い独り言なのでスルーしてもらっていいんですけど.
v-ifとv-modelを呼び出し側で指定し, showUpdateNotificationDialogと isUpdateAvailable をEditorHomeに持ってくることで他コンポーネントと形式を揃えられるとのことだったけど(自分も賛成したけど), 改めて考えたら v-if も v-model も showUpdateNotification も isUpdateAvailable も UpdateNotificationDialog.vue 内で完結させたほうがいい気がしてきた.
というのも, このアップデート通知ダイアログはEditorHomeで呼び出されている他の多くのダイアログコンポーネントと違って, 閉じたり開いたりせず, 閉じる方向しかない. 対して他のコンポーネント, 例えば<help-dialog />や<setting-dialog />はボタンを押すと画面が切り替わったりダイアログが出たり, キャンセルボタンを押すと画面が戻ったりダイアログが閉じたりする. 開いたり閉じたりを繰り返すので<help-dialog v-model="isHelpDialogOpenComputed" />のisHelpDialogOpenComputedはstoreをつかって状態を管理している
const isSettingDialogOpenComputed = computed({
get: () => store.state.isSettingDialogOpen,
set: (val) => store.dispatch("SET_DIALOG_OPEN", { isSettingDialogOpen: val }),
});状態を行ったり来たりしないアップデート通知ダイアログはここまで大げさなことをしなくていいので, EditorHomeではなくUpdateNotificationDialog.vueにshowUpdateNotificationDialog = ref<boolean>(true);って書いておけばいいと思った.
v-if(isUpdateAvailable)に関しては, EditorHomeで他にv-ifが使われているところを参照すると,
- コンポーネントにすらなってない部分で軽く使われてる
<template v-if="isEngineWaitingLong">
<q-separator spaced />
エンジン起動に時間がかかっています。<br />
<q-btn
v-if="isMultipleEngine"
outline
:disable="reloadingLocked"
@click="reloadAppWithMultiEngineOffMode"
>
マルチエンジンをオフにして再読み込みする</q-btn
>
<q-btn v-else outline @click="openQa">Q&Aを見る</q-btn>
</template>- 複数のコンポーネントに渡って使われている
<character-order-dialog
v-if="orderedAllCharacterInfos.length > 0" ←
v-model="isCharacterOrderDialogOpenComputed"
:character-infos="orderedAllCharacterInfos"
/>
<default-style-list-dialog
v-if="orderedAllCharacterInfos.length > 0" ←
v-model="isDefaultStyleSelectDialogOpenComputed"
:character-infos="orderedAllCharacterInfos"
/>って感じなので, 他のコンポーネントでは使わないisUpdateAvailableはUpdateNotificationDialog.vue内にとどめておいたほうがいいような気がした(オタク特y
There was a problem hiding this comment.
たしかに、今のだと関数定義を無駄に散らかしてる感あるね。
UpdateNotificationDialogでしか使わないならUpdateNotificationDialogに入れる方が良いかも（ローカル変数のスコープは最小限にする、と同じポリシー！）。
（liszt01、めっちゃコミットしてくれてて感謝です。）
内容
アップデート可能ダイアログをEditorHome.vueに直接実装するのではなく, コンポーネントにして呼び出すようにした
関連 Issue
スクリーンショット・動画など
その他