| Sharvaniharan | |
| Nov 14 2023, 1:58 AM |
| F41598589: Screenshot_20231213-152946.png | |
| Dec 13 2023, 2:34 PM |
| F41598587: Screenshot_20231213-152927.png | |
| Dec 13 2023, 2:34 PM |
| F41569536: clust.heic | |
| Dec 7 2023, 3:09 AM |
This is an engineering task to:
APK: https://github.com/wikimedia/apps-android-wikipedia/pull/4182/checks
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Dbrant | T347201 [Epic] Bring Places feature back to the Android App | |||
| Resolved | Sharvaniharan | T351159 Places: Build the clustering experience |
Hi @scblr
This is still a work in progress, and only concentrates on clustering experience, no changes to the rest of the map UI. Was wondering if I could get some feedback on:
APK: https://github.com/wikimedia/apps-android-wikipedia/pull/4309/checks
Thanks @Sharvaniharan 👋
Circle radius is not exactly set in dp so just want you to verify if it looks good
Looks good
Should the circle color be themed or just green600 on all themes
Themed (success color) is likely better, since we’re going to use a darker version of the map.
Do we need anything to happen if the user clicks on the cluster circle? If so is that very important or can we skip as they can zoom in anyway 😅
Ideally, yes! Could you explore it and see if possible? It should lead to an unclustered view of that area. See iOS for more details on interaction.
Cool @Sharvaniharan 👏
I have put in the cluster zoom-in. Not as fancy as the apple maps but I think it is working well. :-)
Please check it out again on the same link
Just checked this out, works! A lightweight zoom-in animation would definitely help to convey what happens to the user. Any chance you could timebox an exploration for this? THX!
The colors of the clusters etc., are not yet perfect, but I guess that wasn’t part of this APK.
Hi @scblr
Thank you for reviewing :-)
Just checked this out, works! A lightweight zoom-in animation would definitely help to convey what happens to the user. Any chance you could timebox an exploration for this? THX!
Have created a task for this with low priority: https://phabricator.wikimedia.org/T352940. We can pick this up towards the end.
The colors of the clusters etc., are not yet perfect, but I guess that wasn’t part of this APK.
Correct Robin.. This task was not for the other UI elements on the map, but I would like to at least get the cluster colors right [not the unclustered one yet] with this task. Could you please let me know if it needs changes for cluster color or stroke color or cluster text color:
cluster-background-color: success
cluster-border-color: paper
cluster-border-width: 2dp
cluster-font-style: h3-button
Thanks!
Hi @scblr
I have merged in the animation changes as well. Please take a look again and check if the above design specs for the cluster look good. Thank you!
@Sharvaniharan I just mirrored the designs to my phone and compared it with the APK:
Implementation  | vs | Design  |
The type (h3-button) seems way bigger in the designs. Can you double-check if you applied the right values? 👇
fontFamily: Roboto
font-weight: Medium
font-size: 16sp
line-height: 24sp/1.5em
I updated the font size @scblr , and the rest of the font and line specs, since this is a library component and we so not have full control, it is matched to the closest we could get with the available fonts.
However the circle color on Figma says success_color and the definition we have for success_color is light: green700, sepia: green700 and dark&black: green600
But looks like you are using green600 on light mode on the figma design. Is the implementation ok with success_color?
Thank you :)
@scblr Please note that the APK is now available on a different link (updated on task description) as well.
Thanks @Sharvaniharan
I updated the font size @scblr , and the rest of the font and line specs, since this is a library component and we so not have full control, it is matched to the closest we could get with the available fonts.
Gotcha, it looks good to me now!
However the circle color on Figma says success_color and the definition we have for success_color is light: green700, sepia: green700 and dark&black: green600
But looks like you are using green600 on light mode on the figma design. Is the implementation ok with success_color?
Using the success color group is perfect!
Thank you @scblr . Moving this to Merged and waiting since code review is over on this. Please feel free to move back if you weren't done with review.
