Conversation
To allow support of local KML file, the file name is used as URL and during the parsing of the URL parameter if the url part of the layer doesn't starts with http then it is ignored and not added to the layer list. I also changed a bit the isLoading logic during the parsing of the url parameter for KML. I did this for 2 reasons: 1. Avoid unnecessary spinner which makes the application feeling to be slow 2. In the case of an external KML layer we cannot load the metadata and it would spin forever Due to his isLoading changes I also updated the load kml plugin to only load kml metadata when the kml is not external and it has missing kml metadata. Simplified KMLLayer class constructor. The fileId can always be parsed from the URL (for non external layer). In order to simplify the constructor which has already quite a lots of parameter remove this one and parse it from the URL whenever possible. Same for the isLoading and isExternal, those two can be guessed from the other parameters.
…ade to vue 3.4 This feature has been adopted in Vue 3.4, therefore it is safe to use it now within vue 3.3. I tried to upgrade to Vue 3.4, but it breaks the drawing module ! Apparently Vue 3.4 has a bug when returning a ref from a composable and watch this ref in a component using the composable. In this case the watch is not fire when the ref changes. This was the case where in `useDrawingModeInteraction.composable.js` the returned ref `lastFinishedFeature` did not fire change event on the watcher in `DawingMarkerInteraction.vue`
The menu section logic was as follow:
1. Open active layers section when a layer was added to an empty list
2. The active layers was open at startup only if the list was not empty
3. The active layers and geo catalogue section were close when opening
any other section
In this logic the active layers was never automatically re-opened unless it went
from empty to non empty.
Now the logic has been changed as follow:
1. By default the active layers is always open (at startup)
2. We have two type of sections:
a. singleMode (formerly nonScrollable): when a section is open, all others are
closed
b. multiMode (formerly scrollable): when a section is open, only singleMode
sections are closed
3. When the drawing and advanced tool section are closed, the active layer section
is automatically open
4. When the import file tool is open, the active layer section is open (closing
the advanced tool section)
This allow for a better user experience and feedback when adding drawing and/or
external layers
In order to debug using the browser debugger tool during cypress tests you need to have the source map and for this you need to provide the vite config to the cypress file pre-processor. Also we need to have the server mode and the cypress mode in sync in order to use our environment variable in tests, otherwise the test will get the prod variable while the server is using the dev variables.
The Vue 3 and Vuex documentation specifies that we should only use javascript plain object as reactive object. Unfortunately we don't do this with our active layers that are javascript class and put into the store. For this use case it is quite convenient and changing this would require a big rework. Therefore we keep the class but with a big warning. The main known issue is that the class getter/setter cannot be reactive which leads to subtle bugs hard to find. Reactivity could also have some issues if the class contains side effect methods (method that set properties). Here I removed all getter/setter and added the warning.
PB-95: Added external KML support
PB-100: Manage camera and 3D parameter from the legacy parameters
PB-116: Added text emphasize in search bar results
It can't get the current version running which don't allow to install package because of our package.json strict engine
Node version 18.19 has an issue with strict engines
- Add rounded corner on bottom right - Add shadow This way it looks similar to the menu
This corner is on the edge of the window and don't need rounding.
This icon change size if in down or right mode
defineModal in vue 3.4 has been added to the official feature.
PB-105: Remove menu share spinner and make embed ui similar to import maps
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
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
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)
- 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
…eric_identification PB-269 : GPX/KML generic feature identification
Added menu advanced tool on prod and remove print menu on prod
When changing the language the import external catalog was triggered even if no url was provided which resulted in errors
Avoid error log during language change
Passing run #656 ↗︎Details:
Review all test suite changes for PR #648 ↗︎ |
|||||||||||||||
On the CI this E2E test is sometimes failing in the icon size check in KML payload. Although this commit should not fix this issue, it removes some bad practice of `force: true` flag and make sure that element are visible before clicking which is a good practice (avoiding clicking on element not yet fully rendered and avoiding race condition).
Remove the useless wait on icon, changing icon size don't trigger any new icon request anymore as all icon are loading from the backend using the same size. Remove a wrong closing action, at this time of the test the icon size selector is already closed.
The icon size test did failed time to time. This was due to a race condition. The icon scale in KML is set by openlayer and uses a scale factor based on the icon size. Now if the icon has not yet be loaded during the KML write, openlayer takes a default size of 64 pixel which defer from our icon size of 48 pixel. Due to this the icon scale of a large icon was 2 instead of 1.5. We solve this race condition by waiting on all icons being loaded before doing the icon size changes. NOTE this is not an issue in real life as the save drawing is always delayed at least 2 seconds which let plenty of time to load the icons. In cypress this delay was reduced to speed up testing.
To export a kml, openlayer requires the icon size in order to compute the scale. If the icon has not yet been loaded when generating the style then the icon scale would be computed with a default size of 64px. The last commit tried to solve this issue in cypress test, but unfortunately waiting on the icon to be loaded did not always solved the race condition. I could not find a better way to ensure this as settings the size when generating the icon style. This solve temporarly the issue, but in the future we should get the size from the backend with the others icon metadata.
Fix race condition when exporting/saving kml
pakb
approved these changes
Feb 21, 2024
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.
Test link