Skip to content

BGDIINF_SB-2808 : Time slider#401

Merged
pakb merged 18 commits intodevelopfrom
feat-BGDIINF_SB-2808_time_slider
Jun 16, 2023
Merged

BGDIINF_SB-2808 : Time slider#401
pakb merged 18 commits intodevelopfrom
feat-BGDIINF_SB-2808_time_slider

Conversation

@pakb
Copy link
Contributor

@pakb pakb commented Apr 21, 2023

@pakb pakb changed the title Feat bgdiinf sb 2808 time slider Time slider Apr 21, 2023
@pakb pakb changed the title Time slider BGDIINF_SB-2808 : Time slider Apr 21, 2023
@pakb pakb force-pushed the feat-BGDIINF_SB-2808_time_slider branch from 6ea6620 to 9851c3b Compare May 10, 2023 06:32
@pakb pakb requested review from hansmannj and ltshb May 30, 2023 14:17
@pakb pakb marked this pull request as ready for review May 30, 2023 14:17
@ltshb
Copy link
Contributor

ltshb commented May 31, 2023

The sync between the time slider and the time selector button is not always working.
image

@ltshb
Copy link
Contributor

ltshb commented May 31, 2023

image

@ltshb
Copy link
Contributor

ltshb commented May 31, 2023

For the sync the sliding of the slider works, but if instead of the sliding the slider, you click on the timeline, the slider move but the time selector button rest unchanged to the old value.

@ltshb
Copy link
Contributor

ltshb commented Jun 5, 2023

As discussed with @pakb the time slider is a pre-visualization tool, which means that closing it revert to the previous state and that the year selected from the slider is not synced with the layer year button and layer year in url.

@pakb pakb requested a review from ltshb June 13, 2023 08:59
@ltshb
Copy link
Contributor

ltshb commented Jun 14, 2023

Issue with the swissimage layer
image

@ltshb
Copy link
Contributor

ltshb commented Jun 14, 2023

The cursor on the slider should changed to grab when hovering the slider button and changed to pointer when hovering the time axle.

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.

Code part looks good, just the two issues during manual tests.

@pakb pakb force-pushed the feat-BGDIINF_SB-2808_time_slider branch from 4da5159 to d60321e Compare June 14, 2023 13:03
@pakb pakb requested a review from ltshb June 14, 2023 13:03
@pakb
Copy link
Contributor Author

pakb commented Jun 14, 2023

I have renamed the class that contains information about timestamp and year, as I found it confusing that we had to write timestamp.timestamp many times.
There is also now constants to hold all the magic numbers or timestamp that are used by our backends

@pakb pakb force-pushed the feat-BGDIINF_SB-2808_time_slider branch from d60321e to 896c060 Compare June 14, 2023 14:55
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 some issues with the grab and grabbing cursor

@pakb pakb requested a review from ltshb June 15, 2023 06:14
@pakb pakb force-pushed the feat-BGDIINF_SB-2808_time_slider branch from 874329f to 8a164f7 Compare June 15, 2023 12:56
@pakb pakb changed the base branch from develop to feat_BGDIINF_SB-2957_update_node_lts_version June 15, 2023 12:56
@pakb pakb force-pushed the feat_BGDIINF_SB-2957_update_node_lts_version branch from 10483c3 to e49f68e Compare June 15, 2023 13:21
@pakb pakb force-pushed the feat-BGDIINF_SB-2808_time_slider branch from 8a164f7 to aa6a9a8 Compare June 15, 2023 13:37
Base automatically changed from feat_BGDIINF_SB-2957_update_node_lts_version to develop June 16, 2023 08:27
pakb added 3 commits June 16, 2023 10:28
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)
pakb added 15 commits June 16, 2023 10:28
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)
@pakb pakb force-pushed the feat-BGDIINF_SB-2808_time_slider branch from aa6a9a8 to 40d1265 Compare June 16, 2023 08:29
Copy link
Member

@hansmannj hansmannj left a comment

Choose a reason for hiding this comment

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

Thanks a lot and sorry that I did not really have too much time for reviewing this!
👍

@pakb pakb merged commit f2d436f into develop Jun 16, 2023
@pakb pakb deleted the feat-BGDIINF_SB-2808_time_slider branch June 16, 2023 08:54
@pakb pakb mentioned this pull request Jun 16, 2023
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.

3 participants