-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fixes issue #5841: Nearby pins: Make it easier to understand what the colors mean #5881
base: main
Are you sure you want to change the base?
Conversation
…e5841_hannagnourt
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.
Looks fantastic! I just noted a few minor things, then it should be good to merge.
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginStart="0dp" | ||
android:text="There is a Wikidata item here with no picture yet." |
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.
Would you mind making the text localizable?
See for instance android:text="@string/app_name"
in app/src/main/res/layout/activity_about.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.
Also, sorry to change the wording now, but I think this text would be more actionable for the user: This place has no picture yet, go take one!
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginStart="0dp" | ||
android:text="There is a Wikidata item with a picture here." |
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.
Wording: This place has a picture already
android:id="@+id/textGrey" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:text="The app is checking if the Wikidata item here has a picture." |
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.
Wording: Now checking whether this place has a picture.
Description (required)
Fixes #5841
Tests performed (required)
Tested
prodDebug
on Pixel 8 Pro with API level 35.Screenshots (for UI changes only)
nearby_legend.mp4