Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permission screen #30

Closed
wants to merge 13 commits into from
Closed

Permission screen #30

wants to merge 13 commits into from

Conversation

BrayanDSO
Copy link
Owner

Pull Request template

Purpose / Description

Describe the problem or feature and motivation

Fixes

Fixes Link to the issues.

Approach

How does this change address the problem?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration (SDK version(s), emulator or physical, etc)

Learning (optional, can help others)

Describe the research stage

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@github-actions
Copy link

Message to maintainers, this PR contains strings changes.

  1. Before merging this PR, it is best to run the "Sync Translations" GitHub action, then make and merge a PR from the i18n_sync branch to get translations cleaned out.
  2. Then merge this PR, and immediately do another translation PR so the huge change made by this PR's key changes are all by themselves.

Read more about updating strings on the wiki,

Use transparency to take account of the text background, so it isn't invisible in gray backgrounds
Layout object that can be used to display a permission and get it.

Ideally, it would use material components to deliver a better appearance, but we don't have material themes yet
@BrayanDSO BrayanDSO force-pushed the permission_screen branch 2 times, most recently from 32d855e to 4dc705e Compare July 25, 2023 18:49
Using an enum to manage permissions has some advantages
- can associate permissions with their screens
- is exhaustive, so all permissions possibilities are known statically
- is parcelable, so it can be passed as an activity extra, or a fragment argument

AnkiDroidFolder is converted to a sealed class in order to handle permissionSets
Ideally, it would use material components to deliver a better appearance, but we don't have material themes yet
Simplifies things by not having to handle callbacks or blocking UI elements/background tasks while getting a permission.

Also, it can be easily launched in other screens if necessary, which can avoid crashes like ankidroid#13518

Additionally, adds a better UI/UX for the user
got unused after permissions started being handled by permission screens

With the new permission screens, the "Please grant AnkiDroid the 'All files access' permission to continue" string isn't necessary anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant