-
Notifications
You must be signed in to change notification settings - Fork 4
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
Marker Icons #87
Marker Icons #87
Conversation
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 noted a few styling changes on the map icons. The images definitely look a little pixelated, so update these with higher-res images from Ace. Also, we should maybe look into using svgs (check out this guide).
The map animations are still super jumpy. It seems to work well from a closer zoom level but not so well from far away. https://www.loom.com/share/c1693f7b60af4d4d8825c0653bdc54e9 I'm not sure if it may be better just to not include them at all until we can figure out how to make it smooth.
Minor detail but I'm also getting this weird jumping https://www.loom.com/share/3e8965da9df34afcba89c1660cc70964
Future todo to reduce clutter:
- Only show the marker label at a certain zoom level. If it's too zoomed, out, only show the icons.
components/store/StoreMarker.js
Outdated
return ( | ||
<MarkerContainer> | ||
{focused && ( | ||
<Image source={require('../../assets/images/Marker_Focused.png')} /> |
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.
Hmm probably need higher resolution versions of these
…chen into tommy/markers
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.
Looking good!
I just noticed this issue sometimes where it lets me have 2 stores selected at once. It usually clears after tapping another store again. I'm not sure if it maybe has something to do with setting the default location? Going to go ahead and merge this now, but definitely something to keep an eye on when you make the next map marker updates.
What's new in this PR
Relevant Links
How to review
Next steps
Tests Performed, Edge Cases
Gif