PB-269 : GPX/KML generic feature identification#636
Merged
Conversation
Passing run #616 ↗︎Details:
Review all test suite changes for PR #636 ↗︎ |
|||||||||||||||
Contributor
|
You cannot edit anymore features in drawing mode, strange that cypress tests are passing ? |
ltshb
requested changes
Feb 12, 2024
c178ac6 to
043f2e7
Compare
ltshb
reviewed
Feb 13, 2024
Contributor
ltshb
left a comment
There was a problem hiding this comment.
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.
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
043f2e7 to
f242bfe
Compare
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
f242bfe to
cb519a6
Compare
ltshb
reviewed
Feb 15, 2024
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)
a677595 to
17ed9d2
Compare
ltshb
reviewed
Feb 19, 2024
Contributor
ltshb
left a comment
There was a problem hiding this comment.
Very nice just a few minor comments
ltshb
approved these changes
Feb 19, 2024
- 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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.fillwhile having a drawing on top of it, and clicking on one of the drawing featuresTest link