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

Add Zoom to AoI Button #3247

Merged
merged 1 commit into from
Feb 5, 2020
Merged

Add Zoom to AoI Button #3247

merged 1 commit into from
Feb 5, 2020

Conversation

lossyrob
Copy link
Contributor

Overview

This PR adds a button to the map that zooms to an AOI if one exists.

Connects #3242

Demo

If there is not an AOI set, the button is hidden on the map:

Screen Shot 2020-01-30 at 10 50 15 AM

Once an AOI is selected, the button shows up on the bottom right of the map:

Screen Shot 2020-01-30 at 10 51 01 AM

Here's what happens when you press it:

zoom-to-aoi

Notes

I implemented analogous entries for the SidebarToggleControl control everywhere I found it, as I believe that gave the best coverage for where the new control needed to be referenced (tests, settings, compare map). However a second look on what other areas of the app should be considered from someone with broader knowledge would be great!

Testing Instructions

  • Notice there is no button on the initial page
  • Select an AOI
  • Notice the button with the target icon; hover over to confirm correct tooltip
  • Zoom to somewhere that isn't centered on the AOI
  • Press the button: You're back!
  • "Change area" to clear the AOI. No more button!

@lossyrob lossyrob force-pushed the rde/zoom-to-aoi-control branch 3 times, most recently from b50be44 to 76aeb88 Compare January 30, 2020 17:45
@lossyrob lossyrob changed the title Add Zoom and Center to AoI Button Add Zoom to AoI Button Jan 30, 2020
@rajadain
Copy link
Member

Tested it out, works great. Will take a look at the code shortly.

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

This works well. Left some minor comments on the code, mainly having to do with style and convention.

src/mmw/js/src/core/fitToAoiControl.js Outdated Show resolved Hide resolved
src/mmw/js/src/core/fitToAoiControl.js Outdated Show resolved Hide resolved
src/mmw/js/src/core/views.js Outdated Show resolved Hide resolved
src/mmw/js/src/core/views.js Show resolved Hide resolved
src/mmw/js/src/core/views.js Outdated Show resolved Hide resolved
src/mmw/js/src/core/views.js Outdated Show resolved Hide resolved
Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 looks great, works well!

@rajadain rajadain assigned lossyrob and unassigned rajadain Feb 5, 2020
@lossyrob lossyrob merged commit f5b383e into develop Feb 5, 2020
@lossyrob lossyrob deleted the rde/zoom-to-aoi-control branch February 6, 2020 00:40
@rajadain rajadain mentioned this pull request Feb 20, 2020
11 tasks
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.

2 participants