Change ask permission for user-notification timing. #296
Conversation
Add logging on user-notification process.
| namespace Covid19Radar.Services | ||
| { | ||
| public interface ILocalNotificationService | ||
| { | ||
| public void ShowExposureNotification(); | ||
| public Task PrepareAsync(); |
| public void ShowExposureNotification(); | ||
| public Task PrepareAsync(); | ||
|
|
||
| public Task ShowExposureNotificationAsync(); |
There was a problem hiding this comment.
asyncを前提として、慣例としてメソッド名の末尾にAsyncを付ける。
| { | ||
| base.OnAppearing(); | ||
|
|
||
| await localNotificationService.PrepareAsync(); |
There was a problem hiding this comment.
表示されたタイミングで通知チャンネルの登録と許諾を得る。
OnAppearingがどういう条件で呼ばれるのかは確認したい。OnResumeは別にあるのでOnStartやOnCreateくらいの頻度だと思うものの、あまり頻繁に呼ばれる場合はログの書き込みが多くなりすぎる可能性がある。
There was a problem hiding this comment.
iOSの場合viewDidAppearのタイミングで、画面遷移もしくは初回の画面生成時にHomePageが表示されたら呼ばれています。
Prismで定義されているようですが、実装を読むとXamarin.FormのOnAppearingのイベントにbindingしているだけのように見えるのでxamarinのOnAppearingがそうなっているようです。
https://github.com/PrismLibrary/Prism/blob/master/src/Forms/Prism.Forms/Behaviors/PageLifeCycleAwareBehavior.cs#L13
一方でOnResumeはApplicationのライフサイクルのもののようで、iOSだとバックグラウンドからの復帰で呼ばれます。逆に画面の表示タイミングで呼ばれることはないです。
https://github.com/PrismLibrary/Prism/blob/master/src/Forms/Prism.Forms/PrismApplicationBase.cs#L221
画面生成時のみ表示させたいという場合はInitializeになりそうです。(iOSだとviewDidLoadっぽい)
ENの開始はこのタイミングのようですね。
https://github.com/cocoa-mhlw/cocoa/pull/296/files#diff-0a52fc58c3dfa8eff5b2cb4952766ea63e70fa808d249be88a30edeb0785e7c1L66
There was a problem hiding this comment.
@keiji ログの出力の頻度ってどのくらいが理想っていうのはありますか？↑の調査の通り、iOSの場合画面を切り替えてHomePageが表示されるたびにOnAppearingが呼ばれるようになっています
There was a problem hiding this comment.
ユーザーのストレージに蓄積するものなので、必要最小限が理想です。
そういう意味では、画面が表示される度に不要な処理が走る現在のPullRequestはよろしくありませんので、Initializeに移動します。
| _loggerService = loggerService; | ||
| } | ||
|
|
||
| #pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously |
There was a problem hiding this comment.
asyncキーワードを使っているメソッドでawaitしていないために表示される警告を抑制する。
|
コード的には問題なさそうです |
|
iOSアプリで最初のチュートリアル時にENの許諾を後回しにしてHomePageViewModelを開いた場合、 許諾アラートは連続で呼ばれてもうまいこと表示されるようになっているので基本的には問題ないかなと思います。動作確認しても大丈夫そうでした。 ただちょっとiOS14でのテストしかしていないのでこれはマージしつつ、後で動作確認を行っておきます。また、順番をちゃんと制御できそうであればちょっと実装考えてみます。 |
| @@ -9,7 +9,6 @@ | |||
| using Covid19Radar.Services.Logs; | |||
| using DryIoc; | |||
| using Foundation; | |||
| using UserNotifications; | |||
There was a problem hiding this comment.
別のプルリクにて、AppDelegate内でUserNotificationsを使用する予定になったので削除しないでいただけると嬉しいですmm
…coa into feature/local_notification_initialize
Add logging on user-notification process.
Issue 番号 / Issue ID
目的 / Purpose
通知チャンネルの登録（Android）と、ローカル通知の許諾（iOS）のタイミングがアプリ起動時だと、初回起動時、利用規約に合意していないユーザーからは気が早く見えてしまうので、ホーム画面を表示したタイミングに変更した。
破壊的変更をもたらしますか / Does this introduce a breaking change?
Pull Request の種類 / Pull Request type
検証方法 / How to test
コードの入手 / Get the code
コードの検証 / Test the code
確認事項 / What to check
その他 / Other information