feat: Improve trust plugin dialog design #2420
Conversation
app/src/main/java/app/revanced/manager/ui/screen/settings/DownloadsSettingsScreen.kt
Outdated
Show resolved
Hide resolved
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Offtopic, but why does it say "ReVanced Manager: ..."? |
|
I think the importance of this screen and dialog is enabling and disabling plugins. Trusting them is a secondary task to achieve that main objective. Strings can be renamed to say "Enable" or "Disable" plugin. And the dialog can mention that enabling the plugin can run arbitrary code and need to be trusted as a "side notice" |
| <string name="downloader_plugin_trust_dialog_title">Trust plugin?</string> | ||
| <string name="downloader_plugin_revoke_trust_dialog_title">Revoke trust?</string> | ||
| <string name="downloader_plugin_trust_dialog_body">Package name: %1$s\nSignature (SHA-256): %2$s</string> | ||
| <string name="downloader_plugin_trust_dialog_body">By continuing you\'ve agreed to running external plugins.\n\nDo not allow any suspicious plugin(s) to be installed as they can run arbitrary code.</string> |
There was a problem hiding this comment.
Sounds like youre signing some legal document. Probably different wording would be better here.
There was a problem hiding this comment.
Also, "plugin(s)" can just be "plugins".
Placeholder representation of the app name |
|
I am not sure what you mean by placeholder. What would be written there if others develop plugins? |
|
The app name of the plugin — for example, the APKMirror plugin would be named ReVanced Manager: APKMirror. This accurately represents what the app is and its purpose. If it were just APKMirror, you wouldn't know it is for ReVanced Manager in the app list, and that it isn't actually by APKMirror. |
|
It should probably say "ReVanced: APKMirror downloader" or "Ushie: Some other downloader" |
|
Would Ushie make sense here? It serves no purpose knowing who the developer is, knowing what app it's for is what makes more sense |
app/src/main/java/app/revanced/manager/ui/screen/settings/DownloadsSettingsScreen.kt
Outdated
Show resolved
Hide resolved
* feat: store patched apps * fix: missing string * feat: save patch selection * feat: things * fix: fix broken query * fix: remove redundant `withContext` * fix: fix
This bump `actions/upload-artifact`@v2 to `actions/upload-artifact`@v3
The repository was moved from `revanced-manager-compose` to the main one, which is `revanced-manager`. The organisation's name has also switched to `ReVanced` (used to be `revanced`).
Add paths-ignore to all markdown files, and .idea folder
Show appreciation message for new contributors
oSumAtrIX
force-pushed
the
compose-dev
branch
2 times, most recently
from
2b380b0 to
dd48011
Compare
Set up CI to publish the API library package as well as release the app.
brosssh
force-pushed
the
compose/feat/improve-trust-plugin-dialog
branch
from
ba1271b to
47129cd
Compare
app/src/main/java/app/revanced/manager/ui/screen/settings/DownloadsSettingsScreen.kt
Outdated
Show resolved
Hide resolved
|
Can this be merged? |
|
If the designers are ok with it then yes |
|
What do you think about this? |
I agree. In addition to changing the button to "Enable"/"Disable", I think the other button should be "Cancel" instead of "Dismiss". "Cancel" is already used in other similar dialogs (see the Advanced section in the settings). A new issue is that when using the words "Enable"/"Disable", there is no longer any purpose for the dialog when disabling it. Disabling a plugin inherently means that it will no longer function or run its code. Therefore, I think it would be fitting for the plugins to have a toggle, and the dialog would only appear when the toggle is being enabled. Here is a proposal for the dialog when enabling a plugin: If the goal is to sound somewhat scary, "run arbitrary code" can be substituted with "execute arbitrary code". |
Closes #2061