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

マイグレーション時にダイアログを出せるようにする #1900

Open
sabonerune opened this issue Mar 4, 2024 · 4 comments
Open

Comments

@sabonerune
Copy link
Contributor

sabonerune commented Mar 4, 2024

内容

マイグレーション時にユーザーに報告をしたり選択が必要な場面がある可能性があります。
現状のコードではブラウザ側のコードが複雑になる可能性があります。

ref: #1881 (comment)

Pros 良くなる点

単純なマイグレーションでは済まない設定の変更がしやすくなる。

Cons 悪くなる点

コードの複雑化
Electronとブラウザの両方で機能を両立させる方法が難しい?

実現方法

  • config.initialize()の引数でダイアログを表示できる関数を渡せるようにする
  • migration関数の第二引数に渡す((config, showDialog)=>{みたいな)
  • マイグレーション内でダイアログを出す

この方法で一応ダイアログは表示できるが単純なメッセージでもマイグレーションが止まってしまうか、複数のダイアログが表示されることになる。

その他

ユーザーに確認しないとマイグレーションできない処理というのはいつが必要になるはず…

@Hiroshiba
Copy link
Member

issue作成ありがとうございます!!

単純なメッセージでもマイグレーションが止まってしまうか、複数のダイアログが表示されることになる。

単純なメッセージだけなら、渡す関数にasync lockなどで同時実行不可にすれば複数ダイアログ表示しないようにもできると思います!
ただまあそんなに何度も起こることじゃないでしょうし、ユーザーに確認するために起動シーケンス停止でも問題ないと思います。

@Hiroshiba
Copy link
Member

vscodeみたく、起動してUI表座などをしたあとにダイアログを表示する方針もありかもと思ったのですが、いまいちな気がしたというメモです。

実装はこのが桁違いに難しいと思います。ダイアログ内容が定まるタイミングとダイアログ表示がとても遠いので。
加えて、UIが起動した後にまた再起動しないとなので一長一短かもです。
なのでまぁ起動前のマイグレーションあたりでダイアログ出す方が無難かなと。

@Hiroshiba
Copy link
Member

こちら、おそらくバグに感じられる方がかなり多そう(と言っても多くて数十人とかな気がしますが)に思います。
「なんか設定が消えてる」という考えになるので、VOICEVOIXソフトに対する信頼感が削れる感じです。
優先度を高めに設定します。

とりあえずダイアログが出してあげれば良いと思います。
起動が一時停止しますが、案内がないよりはずっとマシなはず。

@Hiroshiba
Copy link
Member

こちら、結局修正せずにリリースしてしまったので、優先度が下がりました・・・。
ってことで優先度ラベルを変えようと思います。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants