Toggle photo picker switch behaviour and tweak phrases #5250
Conversation
…er UX The enable state used to trigger the GET_CONTENT intent. Alter the flow such that the GET_CONTENT intent is triggered when switch is disabled. Adjust default value and other parts of code naming to reflect this. The existing phrasing had a lot of tech jargon in it which could result in the non-technical users being confused. Tweak the phrasing to avoid such phrases. The documentation in the website could also use some follow up improvements.
Codecov Report
@@ Coverage Diff @@
## master #5250 +/- ##
============================================
+ Coverage 59.86% 59.96% +0.09%
Complexity 2846 2846
============================================
Files 344 344
Lines 17221 17221
Branches 1635 1635
============================================
+ Hits 10310 10326 +16
+ Misses 6013 5993 -20
- Partials 898 902 +4
|
app/src/main/res/values/strings.xml
Outdated
| <string name="get_content_photo_picker_explanation">Disable if your pictures get uploaded without location</string> | ||
| <string name="location_loss_warning">Please make sure that this new Android picker does not strip location from your pictures.</string> | ||
| <string name="open_document_photo_picker_title">Use document based photo picker</string> | ||
| <string name="open_document_photo_picker_explanation">The new Android photo picker has the potential to strip location information. Enable if you seem to be using it.</string> |
There was a problem hiding this comment.
"has the potential to" sounds positive. How about conveying the same in a way that implies it is a bad thing?
Example:
"The new Android photo picker risks losing location information"
Same for the string below.
app/src/main/res/values/strings.xml
Outdated
| <string name="location_loss_warning">Please make sure that this new Android picker does not strip location from your pictures.</string> | ||
| <string name="open_document_photo_picker_title">Use document based photo picker</string> | ||
| <string name="open_document_photo_picker_explanation">The new Android photo picker has the potential to strip location information. Enable if you seem to be using it.</string> | ||
| <string name="location_loss_warning">Turning this off could trigger the new Android photo picker. It has potential to strip location information.\n\nTap on \'Read more\' for more information.</string> |
There was a problem hiding this comment.
If my understanding is correct, toggling the switch would definitely trigger the new Android photo picker right?
If yes, we should modify - "could" to "would" in the following line - Turning this off "could" trigger the new Android photo picker.
There was a problem hiding this comment.
toggling the switch would definitely trigger the new Android photo picker right?
Not always. It will be triggered only when the GET_CONTENT takeover is enabled on a device (and as of now, we do not know how to detect its status).
There was a problem hiding this comment.
Oh then it makes sense!
|
@nicolas-raoul Thanks for the review. I've tweaked the phrasing as mentioned. Do check and merge this if it looks good 🙂 |
|
Sorry for the delay! |
Description (required)
The enable state used to trigger the GET_CONTENT intent. Alter the flow such that the GET_CONTENT intent is triggered when switch is disabled. Adjust default value and other parts of code naming to reflect this.
The existing phrasing had a lot of tech jargon in it which could result in the non-technical users being confused. Tweak the phrasing to avoid such phrases.
The documentation in the website could also use some follow up improvements.
Note: This addresses follow up comments in MR #5227 and does not handle #5242 (that's to be looked into separately).
Tests performed (required)
OnePlus Nord running Android 11
Screenshots (for UI changes only)
cc @nicolas-raoul @Rishavgupta12345 @kartikaykaushik14