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

エディターのメイン画面でアップデートを通知する #1616

Merged
merged 6 commits into from
Nov 3, 2023
Merged

エディターのメイン画面でアップデートを通知する #1616

merged 6 commits into from
Nov 3, 2023

Conversation

liszt01
Copy link
Contributor

@liszt01 liszt01 commented Oct 22, 2023

内容

エディターを起動したときに(新しいバージョンがあれば)アップデートを通知します。

関連 Issue

ref #235

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

Screen.Recording.2023-10-22.at.16.35.17.mov

その他

アップデート通知ダイアログをコンポーネントにして EditoHome.vue に追加してみたのですが、わざわざコンポーネントにするのではなく SHOW_CONFIRM_DIALOGSHOW_NOTIFY_AND_NOT_SHOW_AGAIN_BUTTON を使ったほうがいい気がしています。

また、設定でアップデートを通知するかどうか変更できるようにしたほうがいいとも思っています(まだ実現できていませんが)。

@liszt01 liszt01 requested a review from a team as a code owner October 22, 2023 07:55
@liszt01 liszt01 requested review from y-chan and removed request for a team October 22, 2023 07:55
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.

(コメントしようとしたら間違えてrequested changesにしてしまいました!)

良い感じだと思います!!!
ちょっとこちらでデザインは調整させていただくことになるかもです、ご了承いただければ!

アップデート通知ダイアログをコンポーネントにして EditoHome.vue に追加してみたのですが、わざわざコンポーネントにするのではなく SHOW_CONFIRM_DIALOG や SHOW_NOTIFY_AND_NOT_SHOW_AGAIN_BUTTON を使ったほうがいい気がしています。

よくある「このバージョンをスキップする」ボタンや、あるいは今のバージョンから最新バージョンまででどのような変更が行われるかのリストを表示するなど、ちょっと一般的じゃないダイアログを表示したくなるような気がしています。
であれば今のうちから一般的じゃないダイアログでもいいかもと思いました。

ただまあ今の機能であればSHOW_CONFIRM_DIALOGでも良いかもです。
どちらでもやりやすいように、という感じです!!

また、設定でアップデートを通知するかどうか変更できるようにしたほうがいいとも思っています(まだ実現できていませんが)。

よくあるのは「このバージョンをスキップする」ボタンでしょうか。
スキップする=二度と表示しないなので、一度見たTipsを二度と表示しないようにするためのConfirmedTipsがあって、機能的には近いかもです。
ただこっちは用意されているものだけに対応しているので、どの最新バージョンが来るかわからないバージョンスキップ機能の場合はちょっと実装を変える必要があるかもです。

一旦このプルリクエスト内では最新版があったら何度もダイアログを表示するという仕様にしておいて、後のプルリクエストでスキップできるように目指してみる(あるいはやってみて難しかったら他の方に任せる)というのはどうでしょう?

@@ -0,0 +1,57 @@
<template>
<q-dialog v-if="isUpdateAvailable" v-model="showDialog">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、ちょっとお伝えし忘れてたのですが、useDialogPluginComponentが便利かもしれません。
(このままでも問題ないです!)

return isCheckingFinished.value && latestVersion.value !== "";
});

const showDialog = ref<boolean>(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

必ず通知されるようになってるかも・・・?

Copy link
Contributor Author

@liszt01 liszt01 Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いまのところ、最新バージョンに更新できるのであれば毎回必ず通知するようになっています。
将来的には最新バージョンがスキップされたかどうかをshowDialogの初期値としたいです。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、なるほどです!!v-if="isUpdateAvailable"が入っているのを見逃していました!

ちょっと相談したいことが・・・!

既存でいろんなダイアログがあるのですが、それらの既存のダイアログは開くかどうかを外のコンポーネントが制御しています。
isHelpDialogOpenComputedとかで)
このダイアログも同じように制御するというのはどうでしょう?
その場合おそらくEditorHome.vue内にuseFetchLatestVersionなどを移動し、isUpdateAvailableになったらisUpdateNotificationDialogOpenComputed = trueにするなどの実装になるのかなと・・・!

今の実装でも間違っておらず、既存の実装に合わせて欲しいというわがままなお願いなのですが、もしよければ 🙇

@liszt01
Copy link
Contributor Author

liszt01 commented Oct 23, 2023

コメントありがとうございます!

よくある「このバージョンをスキップする」ボタンや、あるいは今のバージョンから最新バージョンまででどのような変更が行われるかのリストを表示するなど、ちょっと一般的じゃないダイアログを表示したくなるような気がしています。

せっかくなので、どのような変更があったか表示できるようにしたいと思います!
ヘルプ / アップデート情報の変更履歴を表示しているところを参考にやってみます。
https://github.com/VOICEVOX/voicevox/blob/87630200ae7d2ea854210e020140753a02dcfe06/src/components/help/UpdateInfo.vue#L16C11-L21C22

また、設定でアップデートを通知するかどうか変更できるようにしたほうがいいとも思っています(まだ実現できていませんが)。

この機能は #543 のコメントにある、

また、アップデート通知をonにするか、offにするかの機能が少しほしいなと思いました。
background.tsで処理しており、electron-storeの情報が利用できると思うので、SettingDialog.vueの中に、その設定を盛り込んで、利用してもいいかなと思いました。
ただ、これはあくまで拡張的な内容なので、別PRに切り分けた方が良さそうです。

を想定していました。設定 / オプションに、アップデート通知の on/off を切り替える項目を追加しようかなと。
「off にしていれば、新しいバージョンがあっても通知されない」という感じです。
以下のように変更しようと考えています。

// components/UpdateNotificationDialog.vue
const showDialog = ref<boolean>(true);
 const showDialog = computed(() => store.state.NotifyUpdate);

「このバージョンをスキップする」ボタンは私には難易度が高そうなので他の方にお願いしようと思います。。

@Hiroshiba
Copy link
Member

Hiroshiba commented Oct 23, 2023

よくある「このバージョンをスキップする」ボタンや、

せっかくなので、どのような変更があったか表示できるようにしたいと思います!

了解です!!是非!!
最新の更新情報を取得する必要があると思います。こちらのURLで管理しているのでお使いいただければ!
https://raw.githubusercontent.com/VOICEVOX/voicevox_blog/master/src/data/updateInfos.json

もし実装する場合の追加の提案なのですが、このリポジトリをフォークして使っている方のために、このURLを.envファイルなどで指定可能にするのはどうでしょう?
ここのVITE_APP_NAMEみたいな感じでVITE_LATEST_UPDATE_INFOS_URLを定義する感じかなと!
(ちなみにVITE_をつけるのが大事です。VITE_APP_NAMEを全体検索すればどうやったら使えるか分かりやすいかもです。難しかったらまあそのままでも!)

設定 / オプションに、アップデート通知の on/off を切り替える項目を追加しようかなと。
「off にしていれば、新しいバージョンがあっても通知されない」という感じです。

なるほどです!
「このバージョンをスキップする」機能があればあまりオフにしたくならないのかもとちょっとだけ思いましたがどうでしょうか?
(そうしてくださいという意図ではなく、あった方が良いかなかった方が良いか判断を迷っていて、ちょっとご意見聞いてみたい感じです!)

実装する場合は設定の「高度な設定」あたりに属するかなと思いました。
「音声をステレオ化」のコードがとても参考になるかもです!

@liszt01
Copy link
Contributor Author

liszt01 commented Oct 24, 2023

https://raw.githubusercontent.com/VOICEVOX/voicevox_blog/master/src/data/updateInfos.json
このリポジトリをフォークして使っている方のために、このURLを.envファイルなどで指定可能にするのはどうでしょう?

すみません、、この文の意図が掴めなかったので具体的に教えていただきたいです。
最新の更新情報を取得する分には GET_UPDATE_INFOS でできるようですが、「URL」が必要になる場面があるということでしょうか?
更新情報を取得している例 (HelpDialog.vue)

「このバージョンをスキップする」機能があればあまりオフにしたくならないのかもとちょっとだけ思いましたがどうでしょうか?

「このバージョンをスキップする」ボタンには個人的に馴染みがなく、どういうものなのか分かっていないです。
例えば、

最新バージョン 0.14.9 が利用できます!と通知された場合に「スキップする」と以後エディタを立ち上げても再度通知されることはない。
この状態で、最新バージョン 0.14.10 が公開された場合は、再び通知する(最新バージョン 0.14.10 が利用できます!)。

ということでしょうか?

on/off 機能について改めて考えてみたのですが、以下の問題点を思いつきました。

  • アップデート通知を off にしたまま、最新バージョンの存在すら忘れてしまう(更新できることを忘れてしまう)
  • off にしていたアップデート通知を on にしようと思うタイミングがなさそう(アップデートする気になったのなら公式サイトに行くはず)

on/off 機能よりも「このバージョンをスキップする」ボタンのほうがアップデート通知の意義に沿っている機能な気がしてきました!

@Hiroshiba
Copy link
Member

すみません、、この文の意図が掴めなかったので具体的に教えていただきたいです。

あ、なるほどです!
そちらのupdateInfosはアプリがビルドした時点での、そのバージョンの更新情報が入ってます。

voicevox/src/background.ts

Lines 333 to 337 in aaf0576

const updateInfos = JSON.parse(
fs.readFileSync(path.join(__static, UpdateInfosJsonFileName), {
encoding: "utf-8",
})
);

最新の更新情報はネット上にしかないのでそれをダウンロードする必要があるかなという意図でした! 🙇

「このバージョンをスキップする」機能があればあまりオフにしたくならないのかもとちょっとだけ思いましたがどうでしょうか?

最新バージョン 0.14.9 が利用できます!と通知された場合に「スキップする」と以後エディタを立ち上げても再度通知されることはない。 この状態で、最新バージョン 0.14.10 が公開された場合は、再び通知する(最新バージョン 0.14.10 が利用できます!)。
ということでしょうか?

ですです!たぶん想像している通りだと思います。

on/off 機能よりも「このバージョンをスキップする」ボタンのほうがアップデート通知の意義に沿っている機能な気がしてきました!

実装した場合の課題点の洗い出しは手戻り防止になるのでありがたいです!!
「このバージョンをスキップする」機能も、スキップしたバージョンのリストをconfigに保存すれば完了なので、実装に興味があればお声かけいただければ!!

@liszt01
Copy link
Contributor Author

liszt01 commented Oct 26, 2023

アップデート通知ダイアログから最新バージョンの更新情報を見れるようにしました!

更新情報のURLに関してなのですが、.envVITE_LATEST_UPDATE_INFOS_URLを定義して

const latestUpdateInfosURL = import.meta.env.VITE_LATEST_UPDATE_INFOS_URL;

と書いたらundefinedになったので、仕方なくハードコードしています。。

@Hiroshiba
Copy link
Member

まだちょっとプルリクエスト完全に見られてないのですが、コメントだけ・・・!

.envでVITE_LATEST_UPDATE_INFOS_URLを定義して

方針は完全に合ってそうなんですが、不思議ですね・・・。
型としてundefinedになっているということでしたら、↓ここも変えないといけないかもです!
https://github.com/VOICEVOX/voicevox/blob/0346b31020e3590a146f2dcc5980e088992ec16b/src/vite-env.d.ts

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.

良い感じの実装になってきていると思います!!!
もうひと押しでマージできるのかなという雰囲気を感じています、もうしばらくお付き合いいただければ 🙇

@@ -7,6 +7,7 @@
<q-page-container>
<q-page class="main-row-panes">
<progress-dialog />
<update-notification-dialog />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dialogの場所ですが、↓にいっぱいあるのでこの辺りに追加していただけると!

<help-dialog v-model="isHelpDialogOpenComputed" />
<setting-dialog v-model="isSettingDialogOpenComputed" />
<hotkey-setting-dialog v-model="isHotkeySettingDialogOpenComputed" />
<header-bar-custom-dialog v-model="isToolbarSettingDialogOpenComputed" />
<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"
/>
<dictionary-manage-dialog v-model="isDictionaryManageDialogOpenComputed" />
<engine-manage-dialog v-model="isEngineManageDialogOpenComputed" />
<accept-retrieve-telemetry-dialog
v-model="isAcceptRetrieveTelemetryDialogOpenComputed"
/>
<accept-terms-dialog v-model="isAcceptTermsDialogOpenComputed" />

return isCheckingFinished.value && latestVersion.value !== "";
});

const showDialog = ref<boolean>(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、なるほどです!!v-if="isUpdateAvailable"が入っているのを見逃していました!

ちょっと相談したいことが・・・!

既存でいろんなダイアログがあるのですが、それらの既存のダイアログは開くかどうかを外のコンポーネントが制御しています。
isHelpDialogOpenComputedとかで)
このダイアログも同じように制御するというのはどうでしょう?
その場合おそらくEditorHome.vue内にuseFetchLatestVersionなどを移動し、isUpdateAvailableになったらisUpdateNotificationDialogOpenComputed = trueにするなどの実装になるのかなと・・・!

今の実装でも間違っておらず、既存の実装に合わせて欲しいというわがままなお願いなのですが、もしよければ 🙇

src/composables/useFetchLatestVersion.ts Outdated Show resolved Hide resolved
@liszt01
Copy link
Contributor Author

liszt01 commented Oct 28, 2023

ダイアログの制御をEditorHome.vueに移しました。

(ちなみに自分の環境だと、VITE_APP_NAMEもundefinedになります)

@Hiroshiba
Copy link
Member

Hiroshiba commented Oct 28, 2023

(ちなみに自分の環境だと、VITE_APP_NAMEもundefinedになります)

おっとなるほどです!なにか妙なことになってるかもですね…。
ファイル名は.envでしょうか?(.env.productionではなく)

そこ以外思い当たる点がないので、解決が難しそうであればこちらでコード変更させていただこうと思います! 🙇

@liszt01
Copy link
Contributor Author

liszt01 commented Oct 28, 2023

はい。.envで定義していました。

viteについて詳しくないのでそちらにお願いしたいです🙏

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

ちょっとUI周りというかUX周りというかで、よりユーザービリティが高くなるような提案をいくつかしてみました!
もし良さそうなのあれば・・・!

これが完了し次第マージさせていただこうと思います!!

自分用メモ:表示をデバッグするときは、EditorHomeのisUpdateAvailableをtrueにし、UpdateNotificationDialog.vuesemver.ltgtにする

src/components/UpdateNotificationDialog.vue Outdated Show resolved Hide resolved
src/components/UpdateNotificationDialog.vue Outdated Show resolved Hide resolved
src/composables/useFetchLatestVersion.ts Outdated Show resolved Hide resolved
src/composables/useFetchLatestVersion.ts Outdated Show resolved Hide resolved
@sevenc-nanashi sevenc-nanashi linked an issue Nov 1, 2023 that may be closed by this pull request
@liszt01
Copy link
Contributor Author

liszt01 commented Nov 2, 2023

アップデート内容を通知ダイアログに直接表示するようにしました!

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

色々な調整ありがとうございました!!
進行がとてもスムーズで非常にやりやすかったです!!

バージョンごとに通知を切れる仕組みは必須機能だと思うので、ちょっとissue作ってみました。
もしよかったら挑戦してみていただければ・・・!!

他にもいろんなissueがあるのでよかったら是非挑戦してみてください・・・!


一点、.envに設定した変数が利用できない点ですが、こちらで試してみた感じviteの設定がそうなっていることが判明しました・・・。
不正確なことを言って混乱させてしまって申し訳ないです 🙇

↓こちらで @sevenc-nanashi さんが修正してくださいました!

追加で.envの値を使うようにこちらからコミットさせていただきます!

@Hiroshiba Hiroshiba merged commit cd028dc into VOICEVOX:main Nov 3, 2023
7 checks passed
@liszt01 liszt01 deleted the feature/notify-update branch November 13, 2023 11:54
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.

アップデート可能かどうかを通知する
2 participants