Story/map experiment/483 #155
Conversation
Kyubinhan
commented
Sep 22, 2020
•
Kyubinhan
commented
- Create a map component with the control where users can switch base layers and overlays
- Create a map component where users can select weather stations using a lasso tool
- Create a map component that animates the various time slices of layers, specifically weather radar data (but not working smoothly tho)
- Create a map component that we can easily replace the weather station dropdown
|
Kudos, SonarCloud Quality Gate passed!
|
| useEffect(() => { | ||
| /* Create a Leaflet map with a layers control */ | ||
| mapRef.current = L.map('map-with-custom-lasso-button', { | ||
| center: [48.4484, -123.6], |
There was a problem hiding this comment.
if I'm understanding this right, this is where you set that when the map is first displayed, the default view is on Victoria & surrounding area, correct? How do you feel about setting the default view to be further zoomed out so that users can see more of the province from the start? I'm thinking it would reduce the need for zooming out and panning when a user is interested in an area other than the southern Island (which I would guess is the majority of users).
There was a problem hiding this comment.
Agreed, should probably zoom to the entire province by default - that said - I don't think it's an issue for this PR.
| }) | ||
| const tempModelOverlay = getEnvCanadaModelLayer('RDPS.ETA_TT') // 'HRDPS.CONTINENTAL_TT' | ||
| const rhModelOverlay = getEnvCanadaModelLayer('RDPS.ETA_HR') // 'HRDPS.CONTINENTAL_TT' | ||
| // TODO: Render legend img: https://geo.weather.gc.ca/geomet?version=1.3.0&service=WMS&request=GetLegendGraphic&sld_version=1.1.0&layer=RDPS.ETA_HR&format=image/png&STYLE=HUMIDITYREL-LINEAR |
There was a problem hiding this comment.
did you want to leave this TODO in here?
There was a problem hiding this comment.
I'm not sure about pulling this PR in as is.
I feel like we should separate between what we know to be proof of concept / experimental and what we're likely to pull in.
How about only pulling in the station selection part, with just a topo base layer, and putting it behind a feature toggle - that way we can have unit tests run on it (no exclusions required) - and keep all the sample stuff in a separate branch for reference purposes?
| @@ -16,4 +16,4 @@ sonar.tests.inclusions=**/*.test.tsx | |||
| # When sonar-scanner runs as a github action, there's an issue with how it mounts. | |||
| sonar.javascript.lcov.reportPaths=coverage/lcov.info | |||
| # We don't need coverage reports on stories or tests | |||
| sonar.coverage.exclusions=public/**, src/*, src/app/*, src/utils/*, **/*.stories.tsx, **/*.test.tsx | |||
| sonar.coverage.exclusions=public/**, src/*, src/app/*, src/utils/*, **/*.stories.tsx, **/*.test.tsx, src/features/map/* | |||
There was a problem hiding this comment.
How much work / throw-away work would it take to to write some tests for it, and just hide it behind a feature toggle? If our intention is to keep this code functioning and in the code base until such time as we get to it's ticket, then we should have a test for it - otherwise we might as well just keep it on a separate branch? What do you think?
This has me leaning towards just keeping a branch around for reference at a later point?
| @@ -0,0 +1,138 @@ | |||
| import React, { useRef, useEffect, useState } from 'react' | |||
There was a problem hiding this comment.
If we're of the opinion that leaflet doesn't handle animations well - should we not just leave this code out? Or put it somewhere for later reference, but not pull it into the code base?
| useEffect(() => { | ||
| /* Create a Leaflet map with a layers control */ | ||
| mapRef.current = L.map('map-with-custom-lasso-button', { | ||
| center: [48.4484, -123.6], |
There was a problem hiding this comment.
Agreed, should probably zoom to the entire province by default - that said - I don't think it's an issue for this PR.
