-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Provide a selectable list of reasons when requesting deletion of a file #1824
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1824 +/- ##
=========================================
- Coverage 4.14% 4.12% -0.02%
=========================================
Files 223 224 +1
Lines 11197 11241 +44
Branches 1021 1023 +2
=========================================
Hits 464 464
- Misses 10698 10742 +44
Partials 35 35
Continue to review full report at Codecov.
|
"I did not know it would be publicly visible", | ||
"I realized it is bad for my privacy", | ||
"Sorry this picture is not interesting for an encyclopedia", | ||
"I changed my mind, I don't want it to be publicly visible anymore"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late notice, but how about an "Other (please detail on wiki)" reason too, for people who have a legitimate reason we have not thought about? :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I guess these strings should be extracted to strings.xml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely should be in strings.xml :)
I am not sure if there is much difference between "I did not know it would be publicly visible", , "I realized it is bad for my privacy", and "I changed my mind, I don't want it to be publicly visible anymore"? Would it be better to have just one option instead of those 3, that just says "Privacy concerns"? @nicolas-raoul
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great if we could finalize the list of reasons to be shown. The rest of the PR looks good to me and the PR can be merged once the strings have been finalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would keep all three, because:
- "I did not know it would be publicly visible" can not be restricted to privacy only. It also encompasses people who thought it was a kind of game and uploaded a test picture of their wall. A bit like a first-time Wikipedia editor who blanks a page just to see if that works and then freaks out when they realize it actually worked.
- "I changed my mind etc" is not only about privacy, it might be that the uploader did not read the license first. I suggest rewording it to just "I changed my mind".
- In the future, these choices might give us better insight about users behavior. For instance if we have 80% of "I did not know it would be publicly visible" then we might decide to add some explanation within the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will make the required changes !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After rebasing the pr with current master, I am getting Failed to find byte code for com/mapzen/android/lost/api/LostApiClient$ConnectionCallbacks
error. Any idea on how to resolve it without changing mapbox dependencies ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tanvidadu Does it happen even on a clean build?
Also, can you update the PR after rebasing so that we can try reproducing the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @maskaravivek ,This error persists even on a clean build
I have updated the Pull request !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pulled your branch and tried building and running prodDebug
and betaDebug
. The above error didn't occur for me. Can you share the full stack trace?
Eventually, if this error keeps happening for you then we can help in testing and get it merged. :)
de52177
to
af3cabf
Compare
app/src/main/res/values/strings.xml
Outdated
Upload your first media by touching the camera or gallery icon above.</string> | ||
<string name="no_uploads">Welcome to Commons!\n Upload your first media by touching the camera or gallery icon above.</string> | ||
|
||
<string name="deletion_reason_1">I uploaded it by mistake</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename string name
. You can probably use
deletion_reason_uploaded_by_mistake
deletion_reason_publicly_visible
deletion_reason_bad_for_my_privacy
deletion_reason_no_longer_want_public
deletion_reason_not_interesting
} | ||
|
||
private void appendArticlesUsed(FeedbackResponse object){ | ||
reason += ". Uploaded by myself on" + prettyUploadedDate(media); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to strings.xml
@tanvidadu This might be helpful #1943 (comment) |
Thanks @maskaravivek ! |
The PR works perfectly for me. Happy to merge it once one more person is able to test it. I nominated 2 files for deletion using this PR, selecting different reasons. https://commons.wikimedia.org/wiki/File:Elephant_6.jpg |
Provide a selectable list of reasons when requesting deletion of a file
Fixes #1768
Description (required)
Added reasons in drop down menu list
Need to work on UI
Tests performed (required)
Tested on ProdDebug.
Screenshots showing what changed (optional)
{Only for user interface changes, otherwise remove this section. See how to take a screenshot}
Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.