digidem / comapeo-mobile

The next version of Mapeo mobile
GNU General Public License v3.0
5 stars 0 forks source link

add check with navigation when PermissionAudio BottomSheet is open, a… #384

Open bohdanprog opened 1 month ago

bohdanprog commented 1 month ago
bohdanprog commented 1 month ago

@ErikSin here is the reason why we made that in that way. I agreed with that solution; unfortunately, I didn't check all the cases. https://github.com/digidem/comapeo-mobile/pull/375#discussion_r1616286339

Here is solution where we handled all cases. We can change that if it looks good for you. Thanks

ErikSin commented 1 month ago

Sorry im a little confused.

Right now, its just not working. When I give permissions for audio it is still asking me for permission.

bohdanprog commented 1 month ago

@ErikSin
I restored the old functionality and recorded the short video of how it works now. https://github.com/digidem/comapeo-mobile/assets/69891500/9d1e0bf2-8e38-4aec-b583-1d5adde917ff

bohdanprog commented 1 month ago

I think this flow is a little confusing.

When the user sets the permission in the settings, we should close the modal. This can be done in the listener:

useEffect(() => {
    const subscription = AppState.addEventListener('focus', () =>
      Audio.getPermissionsAsync().then(perm=>{
      setPermissionData(perm)
      if(perm.isGranted) closeModal()
      }),
    );
    return () => subscription.remove();
  }, []);

The problem with this, is the hook usePermissions in ThumbnailAndActionTab doesn't get updates. If we do what I suggested above: The user has set permission in the setting, permission is granted and the modal is closed automatically. But inThumbnailAndActionTab permissionResponse === false (because the hook doesn't track when permission are asked outside the app). Since permissionResponse === false, the user will press the audio button, and the modal will open again even though they have the permission.

What needs to happen is the state needs to be tracked in the parent component with one hook const [permissionData, setPermissionData] = useState<PermissionResponse>();

Right now you are using 2 hooks: the usePermission in the parent and useState<PermissionResponse>() in the modal; Intead use useState<PermissionResponse>() in the parent, and pass it values as props to the modal. And then the permissionData will be synced between the parent and the modal, meaning that the modal will close when the setting have been changed AND the parent will know that, and not attempt to open the modal again

Okay, I thought we shouldn't have to close the modal after the user has set permission in the setting and permission is granted I thought we should stay in the same place and after he presses the Allow button we navigate him to another screen. Now it makes more sens to me thanks for your suggestion. I'm going to change that.

ErikSin commented 1 month ago

Okay, I thought we shouldn't have to close the modal after the user has set permission in the setting and permission is granted I thought we should stay in the same place and after he presses the Allow button we navigate him to another screen. Now it makes more sens to me thanks for your suggestion. I'm going to change that.

Just for clarity, what should happen is that when they change their permissions in the setting, the modal should close and the user should be navigated to the audio page. Obviously that is out of scope for this PR, so we should just close the modal for this PR