[health] Fixed request permission to Health Connect for Android

cph-cachet / flutter-plugins

A collection of Flutter plugins developed by CACHET

551 stars 681 forks source link

[health] Fixed request permission to Health Connect for Android #927

Closed iamLuCat closed 7 months ago

iamLuCat commented 8 months ago

Issue: #893

Fix the issue of requesting permission when the Health Connect application hasn't been installed

Video demo

https://github.com/cph-cachet/flutter-plugins/assets/87456448/a27541ba-22b1-4b5f-b88c-c406fe226561

bardram commented 8 months ago

Hi @iamLuCat - I really like this PR, but I've been updating the Health plugin to version 10.0.0 (published), and this PR would need to go into this version. I can't quite see exactly what changes this PR adds and it would be best if you could update this PR to be compatible and mergeable with the current master branch.

ciriousjoker commented 7 months ago

@bardram

This PR conflicts with #941 and #943 and I suggest merging those instead

Differences:

Ready to merge, no conflicts
941 has more flexibility because it exposes the status directly instead of condensing the 3 possible states into a single bool.
Returning only a single bool has the massive downside of requiring DeviceApps.isAppInstalled(), which I see as a much more fragile way of checking for Health Connect availability than asking the Health Connect sdk directly. Also this requires a separate package (DeviceApps), which isn't really necessary
I like the choice in the example code of using a different Flutter package to open the Play Store with the relevant package url, but I saw this too late and by that point I already integrated created the second PR

iamLuCat commented 7 months ago

@bardram I will update the version today. Thank you for your response.

bardram commented 7 months ago

@bardram I will update the version today. Thank you for your response.

@iamLuCat - please see the comment from @ciriousjoker above. As far as I can see, this PR can be replaced by #941 and #943. So - there is no need to do anything here. Right?

iamLuCat commented 7 months ago

@iamLuCat - please see the comment from @ciriousjoker above. As far as I can see, this PR can be replaced by https://github.com/cph-cachet/flutter-plugins/pull/941 and https://github.com/cph-cachet/flutter-plugins/pull/943. So - there is no need to do anything here. Right?

@bardram That's correct. I want to make a slight adjustment on my branch. @ciriousjoker's PR is better.

bardram commented 7 months ago

This has now been released in 10.2.0

cph-cachet / flutter-plugins