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 a default projection (OSMTILE) for the map #873

Merged
merged 62 commits into from
Sep 26, 2023

Conversation

AliyanH
Copy link
Member

@AliyanH AliyanH commented Jul 18, 2023

  • Add default projection of OSMTILE for mapml-viewer
  • remove use of 'createmap' event
  • resolve issues related with custom projection
  • web-map support

Notes
ToDo -

  • [ ] move createAndAddTemplatedLayers from on add in mapmlLayer to process content similar to how we handler other layer types Should be done as part of map-extent custom element development.
  • Add a projection parameter to _convertAndFormatPCRS
  • get rid of 'foo' event

current known issues on experiments (need to re-verify after latest commits) -

  • experiments/painting/ Girl example, clicking on girl does not zoom
  • experiments/alternate-style/ maintian opacity across different styles
  • experiments/linking/features - clicking on BC does not work, Ontario link not working
  • experiments/vector tiles - not working
  • legend does not show up as hyperlink for layers with legend (tried a layer with inline content, legend did not appear)

Other issues/problems found that exist on main also -

Things to do after this PR is pushed -

  • Every time a map-feature is updated update the layerbounds dynamically (make GetBounds update dynamically) Add dynamic layerBounds #892
  • layer.extent needs to be set by the layer- getter which would call the setLayerElExtent
  • create separate classes for FeatureLayer.js, for the different use cases such as statictilelayer, query, templated, etc.
  • featureRenderer - link between custom element and feature renderer, maybe migrate the resetFeature logic to it later

@AliyanH
Copy link
Member Author

AliyanH commented Sep 18, 2023

ToDo -

  • move createAndAddTemplatedLayers from on add in mapmlLayer to process content similar to how we handler other layer types
  • Add a projection parameter to _convertAndFormatPCRS

current known issues on experiments (need to re-verify after latest commits) -

  • experiments/painting/ Girl example, clicking on girl does not zoom
  • experiments/alternate-style/ maintian opacity across different styles
  • experiments/linking/features - clicking on BC does not work, Ontario link not working
  • experiments/vector tiles - not working
  • legend does not show up as hyperlink for layers with legend (tried a layer with inline content, legend did not appear)

Other issues/problems found that exist on main also -

  • ZoomTo from layer control on unchecked layer (get layer unchecked by checking and unchecking a few times) throws an error
  • Need to find a better way of displaying legends, there is already a PR that started some work on this Show Legend Image #763
  • When clicking on a zoom to here link on a popup for a query layer, it zooms to the max zoom of the projection, this does not seem that useful or logical, maybe it should zoom to layer or this link for query layers should not even exist.
  • Give the users the ability to provide a icon for map-point element

Things to do after this PR is pushed -

  • Every time a map-feature is updated update the layerbounds dynamically (make GetBounds update dynamically)
  • layer.extent needs to be set by the layer- getter which would call the setLayerElExtent
  • create separate classes for FeatureLayer.js, for the different use cases such as statictilelayer, query, templated, etc.
  • featureRenderer - link between custom element and feature renderer, maybe migrate the resetFeature logic to it later

@prushforth
Copy link
Member

Tests pass on windows, fail on linux (wsl2) locally, same error messages as on CI, so need to figure that out.

@prushforth
Copy link
Member

The QueryHandler is querying the layer control for the order of map-extent elements, but it should be querying the DOM and / or ShadowDOM for that order, since the layer control may not be in use,only the content of the layer- either in light or shadow root

AliyanH and others added 17 commits September 25, 2023 15:48
Add default projection (OSMTILE) + remove use of 'createmap' events
function declarations to encapsulate and make the whole process a bit
understandable.  Not finished yet, but it builds.

Fix spooky errors due to closure and other unknowns TBD

Tune to closure scopes, add comment, rename attachToLayer to
copyRemoteContentToShadowRoot

Add temporarily named function for dealing with projection matching
Tests not working, some problems with custom projections.
WIP on tests, label, timing

More WIP
Change signature of createLayerControlExtentHTML
Fix broken test mismatchedLayerWithMap test by running _validateDisabled
on extentload.
functions for use in / by _initialize / _processContent
…n test

Essentially what we did before lunch / late Wednesday
…MLLayer per below

Refactor MapMLLayer and layer.js so that fetching is done by layer.js
Temporarily rename MapMLLayer 'extentload' event to 'foo', so as to avoid
collision with 'extentload' event
…er connectedcallback

Comment out testing for now, while things are crazy
Change 'extentload' to 'foo' in map-feature temporarily
Fix mapml-viewer.whenProjectionDefined to remove setTimeout function
that defines failure in case of success.
Add whenProjectionDefined test to setter for projection
created during _initialize, as the content is available via parameter
…ents

to depend on for initialization of parent / required dependencies.
FeatureLayer.js - move code from initialization to onAdd so map 
available
map-features.js - wait until _layer is ready to initialize - 
TODO wait until map-extent is ready
feature.js - temporarily removed creation of an event handler during 
link creation because it was accessing the _layer which didn't exist
TOOD - revisit, figure out how to create event handler without _layer
access
MapMLLayer.js - adapt changeStyle event handler to stop propagation 
of event when selecting different style from option, because it was
accessing the _layer (this is a bug fix for main, too).
web-map.js - copy equivalent changes from mapml-viewer.js
yhy0217 and others added 18 commits September 25, 2023 16:07
…uld be rendered on map or not for features' first time rendering
gets updated by layer control slider and layer- setter.
not set (setter not run), should return the attribute value if _opacity
is nullish.
not set (setter not run), should return the attribute value if _opacity
is nullish.

Update to do list

Work in progress - BNG and Feature Links (BC link) seem to have
mutually exclusive solutions - zoomTo after changing projection on
map is necessary to inform Leaflet of the changed CRS (scale difference)
but the history gets messed up and the BC link doesn't quite render
(you have to generate a moveend on the map, then it'll draw).

Removed a section  of code from _handleLink that sets the map viewer
projection when there is a single map layer and it has a different projection
from that of the map BECAUSE this is handled by the MapMLLayer in
selectAlternateProjection and we don't want to trigger the promise
involved in disconnecting / reconnecting all map layers (happens when
mapml-viewer projection attribute changed callback happens).
…gedCallback is triggered, avoiding the wrong map lat,lng and zoom information read in attributeChangedCallback closure
post-projection change sequence so that history is correctly initialized,
hopefully.

Update to do list
Enable playwright tests, disable jest tests, possibly forever.

Skip featureIndexOverlay tests for now

Do 'npx playwright install' after 'npm install' per playwright error message.
directly at the command prompt, hopefully.

Delete to do list, tracked on PR notes

WIP on getting featureIndexOverlay to behave

Add localized message to feature index output for 'No features found'

Remove commented code

Remove unused / uncovered code, add comments.
Update _handleLink to focus map when finished handling, so that
the feature index shows up again (if enabled).

Ensure feature index and reticle displayed after reload
Ensure feature index and reticle displayed before and after fullscreen

Upgrade playwright

Add tests for feature index and reticle display after fullscreen,
history navigation, link activation.

De-flake a test that got really bad post-upgrade of playwright

Remove layerchange layeradd layerremove overlayremove events from
feature index handlers; the index and reticle display on map focus,
disappear when focus moves off map.  This seems a bit odd when adding
or removing layers, but it is simple, and reticle returns with focus.

Update GeolocationButton to focus the map after it locates the user.
Update geolocation tests to adapt to changes.

Add feature index tests for after reload, fullscreen, history nav,
geolocation activation,

Add test feature index presence post-geolocation activation,
deactivation, and link following actions

Formatting by prettier

Split up slow test

Make feature index overlay responsive to map width

Change x client from npx to npm for ci testing
wait until all results are in before merging into popup interface.

Extend waiting period to 1s when re-ordering extents via layer control,
so that browser processing can finish.
Remove timeout from multipleHeterogeneousQueryExtents test

Remove windows/linux system dependency from test

Fix system dependency for copy-pasting text in test
@prushforth prushforth force-pushed the default-map-projection branch 2 times, most recently from 3697c30 to f8f9109 Compare September 25, 2023 20:42
AliyanH and others added 2 commits September 26, 2023 15:03
…nder to add events in map-feature.js

Intentionally set focus on map viewer at test begin

Attempt to remove system dependencies in tests

Prettier format

Change tests to use https when accessing WMS services

Revert timeout for playwright to 30s
@prushforth prushforth marked this pull request as ready for review September 26, 2023 19:42
@prushforth prushforth merged commit 3aaa9d1 into Maps4HTML:main Sep 26, 2023
1 check passed
@AliyanH AliyanH mentioned this pull request Nov 8, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment