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

Makes depicted place and category items unselectable for nearby place #5325

Merged

Conversation

srishti-R
Copy link
Contributor

Description (required)

Fixes #5317

What changes did you make and why?
Makes the place and category unselectable

Tests performed (required)

Tested {prodDebug} on {pixel 5 emulator} with API level {34, 30}.

Screenshots (for UI changes only)
greyed_check

Need help? See https://support.google.com/android/answer/9075928


Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

I just tested with a Wikidata item that has no associated category (https://www.wikidata.org/wiki/Q105799610 embassy of Brazil in Costa Rica), it seems like a strange "Hidden" category is shown instead of nothing being pre-selected:

screen-20231003-224913.mp4

It seems to work great with Fukusho-ji (which has an associated category).

@srishti-R
Copy link
Contributor Author

I just tested with a Wikidata item that has no associated category (https://www.wikidata.org/wiki/Q105799610 embassy of Brazil in Costa Rica), it seems like a strange "Hidden" category is shown instead of nothing being pre-selected:

It seems to work great with Fukusho-ji (which has an associated category).

I thought this was intended, will remove.

@srishti-R
Copy link
Contributor Author

@nicolas-raoul made the requested changes, "hidden" is no longer a category that appears checked when no category is there. Need some help with 2 failing tests inside CategoriesPresenterTests.kt, I have never written tests for RxJava before and I have used up every brain cell available to fix those two.

@nicolas-raoul
Copy link
Member

Note: Just tested, it works great now, thanks! I did not actually upload any file but I will try to, probably in the next few days.

About the unit tests: Would you mind creating a new issue about it? Please include Git repo/branch info to make it easily reproducible. Thanks! :-)

@srishti-R
Copy link
Contributor Author

@nicolas-raoul I was able to fix all the CategoriesPresenter failing tests as here #5327. One confirmation I needed to make this deselectable feature better. Suppose we upload a set of images that have exif attached, we confirm by saying "Yes" to "is this a photo of some place nearby?", after this, shouldn't the depiction and category also be unselectable? Currently its not. We can choose to select/unselect the depiction and category in this specific case.

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Tested again, it still works great (I did not actually finish the upload though as I did not have suitable pictures)

@nicolas-raoul nicolas-raoul merged commit 733c870 into commons-app:main Oct 8, 2023
1 check passed
@nicolas-raoul
Copy link
Member

When using Nearby, the app does assign the uploaded picture to the Wikidata item. Even if the user changes their mind they have no way to prevent this assignment from taking place. So if they change their mind, they MUST just cancel their upload.

On the other side, even after selecting "Yes" for "is this a photo of some place nearby?", the user can still modify everything (captions/descriptions/depictions/categories), and it will be like if they has selected "No". The Wikidata item is not modified.

Summary: Nearby upload MUST be about that Wikidata item. Normal upload does not have such restrictions.

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.

When uploading via Nearby, make the item's depiction (and category) undeselectable
2 participants