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

#5740 - Add Welcome Dialog Explaining Permissions #5848

Merged

Conversation

david-allison
Copy link
Member

Purpose / Description

Some users din't feel comfortable about us requiring access to external storage.

Fixes

Fixes #5740

Approach

We add a dialog explaining that we don't perform any malicious activities with this in order to hopefully alleviate concerns.

How Has This Been Tested?

image

Checklist

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code

@david-allison
Copy link
Member Author

Wording changes would be useful. Probably "storage access" -> "storage permissions"

Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor wording change, suggested re-ordering of ok click handler

AnkiDroid/src/main/res/values/03-dialogs.xml Outdated Show resolved Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.java Outdated Show resolved Hide resolved
@mikehardy
Copy link
Member

I blew up master CI by merging #5597 which had passed long ago before we included a lint check, and it fails the lint check post merge 🤦‍♂ - just pushed a fix for that and master should go clean but you'll probably have to rebase from master HEAD and force-push to kick CI again

Then unrelated Travis did some sort of maintenance and macOS builds are super intermittent right now, but those I can keep kicking until they go green...

@mikehardy
Copy link
Member

One separate question before I approve, what happens on a tiny screen? The API16 emulator I keep around has two purposes - one it verifies the oldest API we still can sync ankiweb too, the other is that I have it configured as an old + tiny screen phone (Nexus S IIRC). Just wondering if there is enough real estate for the whole message

@david-allison
Copy link
Member Author

One separate question before I approve, what happens on a tiny screen? The API16 emulator I keep around has two purposes - one it verifies the oldest API we still can sync ankiweb too, the other is that I have it configured as an old + tiny screen phone (Nexus S IIRC). Just wondering if there is enough real estate for the whole message

Sorry, forgot that in the testing notes. It works fine with a scrollbar.

Some users din't feel comfortable about us requiring access to
external storage.

We explain that we don't perform any malicious activities with this
in order to hopefully alleviate concerns.
@david-allison david-allison force-pushed the 5740-dialog-on-permission-request branch from 9999a0c to d073c5c Compare March 21, 2020 17:40
@david-allison
Copy link
Member Author

Rebased, squashed and pushed.

Let's hope the CI still passes 🤞

@timrae
Copy link
Member

timrae commented Mar 21, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 1
           

See the complete overview on Codacy

@mikehardy mikehardy merged commit bf4ce65 into ankidroid:master Mar 21, 2020
@mikehardy
Copy link
Member

Sweet - this will go in alpha53 later today

@mikehardy mikehardy added this to the 2.10 release milestone Mar 21, 2020
@david-allison david-allison deleted the 5740-dialog-on-permission-request branch March 21, 2020 20:25
@mikehardy
Copy link
Member

2.10alpha54 released just now has this whenever google delivers it to you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display a dialog explaining why permission is necessary before requesting external storage
3 participants