Conversation
when set to true, the UI is simplified to a minimum (zoom in zoom out and a link to open the app in a new tab) I had to rework a bit the URL parsing tooling for this, as otherwise the embed=false or embed= were staying in the URL. But working on this opened another whole can of worms with app startup and I had to change the logic in some places in the storeSync plugin to fix that (layers and bgLayers were disappearing at startup somehow)
Insead of choosing between keeping an empty value in the URL or not, we now can define which is the default value of a param, and if it should stay in the URL when the value in the store is the default value.
BGDIINF_SB-2501 : add support for embed URL param
even if deployed on the test viewer to mitigate any inconvenience to the user I've also lowered the greyscale of the font, making this version number more subtle and difficult to spot.
Now that we support embed=true URL param (with specific UI for this) we can add back the iframe snippet generator portion of the UI with some minor adjustments : - short link used by the iframe snippet generator is a different one from the share section, as it requires to have embed=true as a URL param - some of the structure made a couple month ago was not up to date with the latest changes I also added some e2e Cypress test to cover this iframe snippet modal and its functionalities
- Switch button icon (for embed preview) from "+/-" to caret as on other menu items - rename generateShortLink function as plural, as there is now two short links being generated - better handling of short link backend error, with fallback to non shorten URL in case of error - removing unnecessary complexity with URL param parsing at store init in ui.store.js, a new task will be created to tackle app startup in embed mode so that it never shows the header bar
so that it mimics the behavior of the short link input
Harmonizing margin after icon in the menu tray so that the sections and the "embed" button have the same look Changing the cursor to a pointer when hovering the size selector in the embed popup
Now only embed is added to the url (without the =true part) so the test was failing to detect correctly the backend request to create a short link with the flag Also renaming all actions from the share.store.js file to plural when they are dealing with both the normal short link and the embedded version
…erator BGDIINF_SB-1856 : share - iframe code snippet generator
updating all libs to latest version
meaning Node 18 and npm 9
as sometimes start-server-and-test wasn't able to catch properly that the server was started as it was listening on 127.0.0.1 and not on localhost. With --host there, vite server will also respond to requests on 127.0.01 correctly
as I'm testing some changes in the buildspec.yml I need the CI to run without any changes in the code...
…e_lts_version BGDIINF_SB-2957 : update node LTS version
And replace the enum declaration by exporting each coordinate system separately (plus all possible systems in an array) This should enable us to check the type of a projection/coordinate system when we receive them through a Vue prop (it wasn't possible to type check with the enum declaration, it's now the case as it is declared as a class)
As there are some layers that uses the first day of the year to describe the full year, and others the last day of the year, I thought it would be easier to switch to a "only-year" notation. This also prepare the grounds for a complete switch to a more OGC compliant approach, with ISO 8601 timestamps (and ranges). Meaning when we only give a year as YYYY, we want all the data from this year (from YYYY-01-01 to YYYY-12-31)
As with a preview layer, this year takes precedence over any other configuration in layers. This will help when we will want to add a time slider, as we don't want the time slider to alter all active layers config, but only "preview" the year that was chosen through the time slider. This means that all URL generation for layers now have to take into account that, if a preview year is defined, they have to replace any timestamp found in the layer's config with the timestamp corresponding to the preview year. I've also simplified some props/data transfers between the OpenLayersInternalLayers and all specialized layers.
so that we have one less warning in the JS console, as it was a bad practice to actively listen to mouse events (creates lag on user inputs)
As it is now only storing the year part of the timestamp. Also double proofing the selected timestamp in WMS and WMTS layer URL definition, with fallback to the timestamp defined in the time config if the one given as preview year is invalid in the context of the layer
Otherwise the UI will still show the preview year in the menu, while the user has chosen to change the layer config. This is about the same behavior as mf-geoadmin3, going a little further by hiding the time slider if this happens (somehow the time slider was kept in mf-geoadmin3, even though it doesn't make much sense as years are now mixed up and do not follow what is selected with the time slider anymore)
As the store was not dispatched anytime the preview year of the time slider is changed (at startup we do not want a dispatch). So I've moved all set + dispatch into a single method that is also called when a click occurs on a year (watching the prop wasn't an option for the reason stated above, at startup we want to set it up without dispatching the store)
As it was confusing to show the change of preview year here, but then not highlighting the previewed year if the year selector popup was opened. And as we want the time slider to be a "preview only" feature, it wasn't making much sense to change the year shown in the menu, as the year in the URL wasn't affected by the year selected through the time slider.
some occurrences of the old "series" name were left unchanged
storing the timestamp "as is" simplifies this logic significantly (no need for this getter and enum anymore)
As it was confusing to have to write timestamp.timestamp, so I've opted for a timeEntry name that contains the year and equivalent timestamp. In the process, I've tried to simplified the case of no time entry for a WMTS layer, and replaced all the "magic numbers" (9999, 'all', 'current') with constants that are imported when needed. I also fixed an issue when the time slider was dealing with a WMTS layer that had a 'current' timestamp available (as it is a different value than the 'all' from WMS it was required to manage this case too)
It was shown below the header bar when the layer from which it comes was sufficiently high in the menu tray, hiding the couple first row of possible year to choose
Cursor state is changed according to the user action or mouse position : - pointer on the year line (as it can be clicked through here) - grabbing hand over the grabbing part of the year selector - the hands change shape to the grabbed position when we click on the grabbing handles
so that when the user drags the year cursor (and his pointer is out of the year cursor) there is consistency with cursor type (it stays grabbing)
hansmannj
approved these changes
Jun 16, 2023
Member
hansmannj
left a comment
There was a problem hiding this comment.
Merci for the hughe efforts! 👍
Contributor
Author
|
CI wasn't able to build properly because it misdetected my feature branch |
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.
Features/Tasks
#407 BGDIINF_SB-2501 : add support for embed URL param
#408 BGDIINF_SB-1856 : share - iframe code snippet generator
#401 BGDIINF_SB-2808 : Time slider
Chores
#410 BGDIINF_SB-2957 : update node LTS version
Test link