added globe and location buttons to map #3913
Conversation
onearmy-community-platform
|
||||||||||||||||||||||||||||
| Project |
onearmy-community-platform
|
| Branch Review |
pull/3913
|
| Run status |
|
| Run duration | 05m 07s |
| Commit |
|
| Committer | Matúš Motyka |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
76
|
| View all changes introduced in this branch ↗︎ | |
Updated tests
prettier fixes
test fix
added click for redirect to the new map
changed comparing scale to comapring values in matrix
fixeees
final fixes?
prettier fix
|
hey @joperafe , I'm not sure if you'll be the one reviewing this or if someone else will, but I think it's finally done. I fought with the tests for a while but eventually found a way to check the zoom. Let me know if any changes are needed or if something is wrong. Thanks! |
|
or maybe @benfurber should review this, not sure hah |
Hi there! No no, not me. First timer Thank you |
benfurber
added
the
Review allow-preview ✅
There was a problem hiding this comment.
Thank you so much for your contribution @motuz0001!
One. Can you move the buttons to the top right please. You'll see why below!
Two. There's a lot of custom styles. I'm hoping that when you don't have to position absolute, most of them go away. I'd rather less custom styling that leave the buttons imperfect.
Three. The tooltips don't work for me on Firefox, do they for you?
Thank you for struggling with the feature test, I appreciate it.
| }, | ||
| }} | ||
| onClick={() => { | ||
| setZoom(1) |
There was a problem hiding this comment.
There's an existing INITIAL_ZOOM const on 1Maps.client` that can be used rather than 1.
|
|
||
| {/* Location button to Zoom in to your location */} | ||
| <Button | ||
| data-tip="Zoom in to your location" |
There was a problem hiding this comment.
I'd rather have...
const ZOOM_IN_TOOLTIP = "Zoom in to your location"
export const MapWithList = (props: IProps) => {
...
<Button
data-tip={ZOOM_IN_TOOLTIP}
Make sense?
|
|
||
| {/* Globe button to Zoom out to world view */} | ||
| <Button | ||
| data-tip="Zoom out to world view" |
|
Oh and a complex rebase is needed! |
PR Checklist
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The map currently does not have a buttons for unzooming to global view and zooming to user's location.
What is the new behavior?
Does this PR introduce a breaking change?
Git Issues
Closes #3879
What happens next?
Thanks for the contribution! We try to make sure all PRs are reviewed ahead of our monthly maintainers call (first Monday of the month)
If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.
If you need more immediate feedback you can try reaching out on Discord in the Community Platform
developmentchannel.