Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

Update Plan wizard are step to use OpenLayers #1356

Merged
merged 7 commits into from
Nov 20, 2019

Conversation

maurizi
Copy link
Contributor

@maurizi maurizi commented Nov 19, 2019

Overview

Update Plan wizard are step to use OpenLayers. This will enable us to visualize layers Google Maps is unable to support, most notably vector tile layers.

Demo

image

Notes

Testing Instructions

  • scripts/update
  • The plan page should continue to function as before, but with a new Google-free basemap
  • The map page and map modal should continue to function as before
  • [ ] Is the CHANGELOG.md updated with any relevant additions, changes, fixes or removals following the format of keepachangelog?

Closes #1335
Closes #1301

Copy link
Contributor

@flibbertigibbet flibbertigibbet left a comment

Choose a reason for hiding this comment

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

The new maps looks nice, and interact well!

I noticed a few things in testing this:

  • toggling to the plan 'Draw Area' tab only displays the map the first time; on subsequent toggles it's blank
  • clicking the 'redraw' button while drawing causes the button to disappear until page refresh, and doesn't remove the shape in progress
  • Loading the dashboard page for an organization with related map data for at least one related impact results in console errors:
    image
  • console error on opening map modal:
    image
  • the scroll size is off in the map modal, cutting off the bottom of the sidebar text:
    image

Probably unrelated to this PR, but going to the plan page with an area already set for the organization doesn't zoom to fit the existing polygon.

COPY ./ngx-openlayers-1.0.0-next.11.tgz /opt/planit/angular/planit/
# Contains the changes https://github.com/quentin-ol/ngx-openlayers/pull/244
RUN yarn add file:./ngx-openlayers-1.0.0-next.11.tgz
COPY ./ngx-openlayers-1.0.0-next.13.tgz /opt/planit/angular/planit/
Copy link
Contributor

Choose a reason for hiding this comment

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

🎊

@maurizi
Copy link
Contributor Author

maurizi commented Nov 20, 2019

@flibbertigibbet I think I've addressed all the issues you found.

Probably unrelated to this PR, but going to the plan page with an area already set for the organization doesn't zoom to fit the existing polygon.

This one is unrelated/intentional - I noticed this too, but it is only possible for this to happen if you reload the page while going through the Plan wizard, which seems likely to be uncommon, so I didn't think it was worth our time to address it.

Copy link
Contributor

@flibbertigibbet flibbertigibbet left a comment

Choose a reason for hiding this comment

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

This is looking better! The console errors are gone. A couple of notes:

  • on toggling between draw and autocomplete tabs in the plan wizard with a polygon already set, the map now always displays, but the polygon only shows the first time the draw tab is visited
  • the map modal is looking better, but still can get cut off when in a short screen:

image

@maurizi
Copy link
Contributor Author

maurizi commented Nov 20, 2019

@flibbertigibbet I've fixed the issue with bounds disappearing when switching tabs.

I was able to reproduce the issue with the map modal styling - I'd like to defer fixing that for now and see if #1342 fixes it, or tackle it in a later styling pass.

Copy link
Contributor

@flibbertigibbet flibbertigibbet left a comment

Choose a reason for hiding this comment

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

👍

@maurizi maurizi merged commit 621d457 into develop Nov 20, 2019
@maurizi maurizi deleted the feature/mvm/org-wizard-ol branch November 20, 2019 20:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants