revanced-manager
revanced-manager copied to clipboard
ReVanced
feat: Add installer status dialog
This PR shows a native dialogue after the installation finishes. It can handle all status flags. Business logic may move.
Todo
- [ ] Handle errors such as:
- [ ] Installing regularly while a mount installation is currently active (In this case, uninstall the mount installation)
- [ ] Mount errors such as wrong base APK or no base APK present (In this case, show an appropriate dialog)
Review
- [x] Check if https://github.com/ReVanced/revanced-manager/pull/1473/files#diff-bd497764717bba99312a9c9db9f60eaa4c5d090266194189db6855574572564bR55 is placed in a suitable location
- [ ] When uninstallation is necessary, first the uninstallation prompt is shown, then the installation prompt
- [x] All strings are chosen appropriately
- [x] All icons are chosen appropriately
- [x] ~~Showing a SUCCESS status dialog is wanted~~
- [x] All TODOs in this PR are considered or done
- [x]
uiSafehas been used correctly (In some places, it seems it should be used but it seems out of scope for this PR)
[!WARNING] On installation, the install button is not greyed out, and no progress is visible, so it appears as if nothing happened.
Is this also for when you update the Manager?
No it's only for installing patched apps
ReVanced Manager Updates will also use this, but this PR focuses on patched apps
@ Axelen123 Currently, the extra status message is not used, but I don't know which extra status messages exist so that I can implement them. For example, if an existing installation is present, but the patched app mismatches signatures, then the dialogue will say:
This flag is returned from installation, and there is no way to differentiate between an APK being invalid, for example, or a signature mismatch.
Furthermore, when uninstalling, the dialogue may also be shown. But once again, there is no way to differentiate between installations, uninstallations or the status message, so the dialogue may say that the installation has failed while the uninstallation failed.
https://github.com/ReVanced/revanced-manager/assets/13122796/aba7c095-1867-4ccc-9fc3-a6a8c4c1e63d
(The recording is cut off at the end, it shows the success dialogue when installation succeeded)
@ Axelen123 Currently, the extra status message is not used, but I don't know which extra status messages exist so that I can implement them. For example, if an existing installation is present, but the patched app mismatches signatures, then the dialogue will say:
This flag is returned from installation, and there is no way to differentiate between an APK being invalid, for example, or a signature mismatch.
Not sure why Android says the app is invalid. Android should give you CONFLICT on signature mismatch. What is the content of the EXTRA_STATUS_MESSAGE here?
Furthermore, when uninstalling, the dialogue may also be shown. But once again, there is no way to differentiate between installations, uninstallations or the status message, so the dialogue may say that the installation has failed while the uninstallation failed.
Isn't it possible to make a separate dialog or enum for uninstalls?
Isn't it possible to make a separate dialog or enum for uninstalls?
Yes, but ideally we would have the existing Flags enum have variants of the install and uninstall strings.
@Axelen123 I have updated the PR description with up to date information.
Please send a recording of the latest changes or link me to them, thanks 👍
@PalmDevs
https://github.com/ReVanced/revanced-manager/assets/13122796/48e874e6-84a4-48cf-9cd4-f9be7442aded
I'll need some help regarding the pending TODOs in the PR body. I am unsure where to get context about root installations when doing regular installation. The idea is, that other dialogs are added in case root installations exist or a root installation can't be done.
@oSumAtrIX
When uninstallation is necessary, first the uninstallation prompt is shown, then the installation prompt
Not sure this is within the scope of the PR. One of the cases requires checking the signature of the installed app, which would require functionality that isn't currently present in revanced library.
Mount errors such as wrong base APK or no base APK present (In this case, show an appropriate dialog)
Isn't this already handled?
Not sure this is within the scope of the PR. One of the cases requires checking the signature of the installed app, which would require functionality that isn't currently present in revanced library.
The dialog model has two functions for installing and uninstalling apps. The installer returns a result at which the dialog shows the corresponding dialogue. If the signature mismatches, it will suggest to uninstall the app using the models uninstall function and attempting the installation again, which, the dialog will handle the result of:
https://github.com/ReVanced/revanced-manager/assets/13122796/48e874e6-84a4-48cf-9cd4-f9be7442aded
What I meant with that message is the issue shown in the recording which shows that the installer runs before the uninstaller. The cause is not known and its unclear if it is an issue outside my env.
Isn't this already handled?
The dialog currently ONLY handles Android installer status. It has no context about root installations and root installers also don't return any status code like the Android installer. As it is right now, the dialog is only handling Android installations.
OK, I think this may be done. Though probably something will come up in review.
Oh, I was waiting for someone to review / confirm them before ticking them.
Oh I just realized, while installing should the installation button be greyed out?
However, the success dialogue will not be implemented right?
Success dialog won't be implemented, as that would be indicated by the "Open" FAB and Android Package Installer's own success dialog
Oh I just realized, while installing should the installation button be greyed out?
It should disappear, there's no concept of a disabled FAB