Skip to content

PB-269 : GPX/KML generic feature identification#636

Merged
pakb merged 15 commits intodevelopfrom
feat_PB-269_gpx_kml_feature_generic_identification
Feb 20, 2024
Merged

PB-269 : GPX/KML generic feature identification#636
pakb merged 15 commits intodevelopfrom
feat_PB-269_gpx_kml_feature_generic_identification

Conversation

@pakb
Copy link
Contributor

@pakb pakb commented Feb 9, 2024

Enhancing how the tooltip is placed for geometry based features, at the most southern point of the most southern feature (also work the same way for points, but that is less obvious)

Should also fix WMS/WMTS feature identification when returned features have a complex geometry instead of a single point.
Fixing a small bug with time sensitive layer where features from all years were requested at the same time, now filtering taking the selected year into account.

A good example to test all this would be ch.swisstopo.swissboundaries3d-gemeinde-flaeche.fill while having a drawing on top of it, and clicking on one of the drawing features

Test link

@pakb pakb requested a review from ltshb February 9, 2024 13:55
@cypress
Copy link

cypress bot commented Feb 9, 2024

Passing run #616 ↗︎

0 176 21 0 Flakiness 0

Details:

PB-269 : only load drawing module when KML data is present
Project: web-mapviewer Commit: 12e6d26a1c
Status: Passed Duration: 04:53 💡
Started: Feb 20, 2024 10:56 AM Ended: Feb 20, 2024 11:01 AM

Review all test suite changes for PR #636 ↗︎

@ltshb
Copy link
Contributor

ltshb commented Feb 12, 2024

You cannot edit anymore features in drawing mode, strange that cypress tests are passing ?

@pakb pakb force-pushed the feat_PB-269_gpx_kml_feature_generic_identification branch from c178ac6 to 043f2e7 Compare February 13, 2024 09:01
@pakb pakb requested a review from ltshb February 13, 2024 09:11
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

Looks promissing, I've found an issue that is not covered by the E2E test (might be good to add test cover):

  • Create a drawing with an icon
  • Reload the page (loosing the adminId)
  • Reopen the drawing => should create a copy of the previous drawing
  • select the existing icon => now its tooltip is not displayed, new icon have the tooltip but not copied icon.

pakb added 10 commits February 15, 2024 10:37
As we will then add a new one, or heavily modify the SelectableFeature class, to implement our generic KML/GPX feature identification
instead of only placing markers for points.

The code that manages a ol/Layer/Vector has been moved from the marker component to a composable, so that it can be used by the component that will highlight features.
as it will then be used in the highlighting of feature to lookup for the most southern feature of the selected
it will be used in both cases, and this way we do not have to declare a third type of selectable features (GeometryFeature or something in this vein), most of the code will stay the same whilst supporting selection of geometry
keeping all features visible in the process. The rotation of the map is rarely not pointing north, so I though it wasn't making much sense to take the rotation of the map into account while calculating this point, but that could easily be achieved by computing the point we give to the nearestPoint function, and moving it according to the map rotation
We return a LayerFeature instead of the specialized one for drawing, making our feature identification compatible with external KML in the process.

Also changing how the popup for KML feature is built, and using the layer title (common practice between layers) and trying to find a description for this specific feature. If no description is given, fallback to a "no further information" text instead of leaving the popup empty.
All features from all years were requested, with this new param only the selected year's features will be returned
so that only const and let keyword are permitted
also fixing an issue with drawing and the way we now select the popover coordinate, passing the drawn geometry directly into the feature (even when drawing, before it was using the coordinates directly without knowing what kind of geometry it was dealing with)
so that they are present to place the popover when we click on a feature while drawing
@pakb pakb force-pushed the feat_PB-269_gpx_kml_feature_generic_identification branch from 043f2e7 to f242bfe Compare February 15, 2024 09:38
let our backend data through without sanitization, otherwise layer such as public transportation stops won't work anymore, as their iframe code is removed by the sanitizer
@pakb pakb force-pushed the feat_PB-269_gpx_kml_feature_generic_identification branch from f242bfe to cb519a6 Compare February 15, 2024 09:50
@pakb pakb requested a review from ltshb February 15, 2024 10:49
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

Still an issue in drawing, when entering the drawing mode with an existing drawing if you select first an existing feature you have an error and no tooltip, we should cover this in e2e test.
image

pakb added 2 commits February 15, 2024 16:19
there was two "ways" (code double) that were parsing coordinates from a OL feature (it was named KML feature in one case, but was a OL feature nonetheless)

I also removed the loading of KML data at drawing startup, using instead the data already loaded through the new KML/GPX pre-load mechanism. This removes the flickering of KML features when going in and out of the drawing mode
We didn't detect that there was issues with the popover placement because tests are only performed with a mobile viewport, meaning the popup is placed in the infobox at the bottom of the screen, without the computation for a over-the-map placement (where the issue was located)

So I've added a test that toggles the floating of the edit popup for a marker and a line (as the calculation is a bit different for these two types when floating)
@pakb pakb force-pushed the feat_PB-269_gpx_kml_feature_generic_identification branch from a677595 to 17ed9d2 Compare February 19, 2024 09:00
@pakb pakb requested a review from ltshb February 19, 2024 09:49
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

Very nice just a few minor comments

- Switch to infobox feature detail as soon as a geometry is detected, this should provide a good-enough solution when we select features that are so big on screen that the tooltip is outside of the current map viewport
- Adding a check when loading the drawing module so that it fails if no KML data is present (and retry later)
at startup, if a KML is present with an adminID (meaning we open straight the drawing module), we wait for the data to be loaded by the store plugin before loading the drawing module
@pakb pakb merged commit 9cd7e32 into develop Feb 20, 2024
@pakb pakb deleted the feat_PB-269_gpx_kml_feature_generic_identification branch February 20, 2024 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants