Conversation
2cd137c to
1f8b5ed
Compare
Passing run #847 ↗︎
Details:
Review all test suite changes for PR #677 ↗︎ |
|||||||||||||||
ltshb
reviewed
Mar 4, 2024
Contributor
ltshb
left a comment
There was a problem hiding this comment.
Huge code improvement, thanks a lot.
and deconstruct its content when called, should shorten most calls to constructors that were using many default/blank/null values to reach one last argument and set it to something else Adding the updateDelay param to GeoJson layer (it will be relevant only relevant for this type of layers)
calculated once and for all at layer creation
we had once some attributes that were getThisId or someUrl, and other that used getThisID and someURL... I thought it would be easier to navigate all this if we sticked to a single way of naming thing, hence the move to baseUrl instead of baseURL
hopefully it is a bit clearer what this is about with this name
checking that what has been deconstructed from the single Object entry is sufficient to go on building the instance. Also moving the timeConfig directly in GeoAdminLayer, so that it may be set to any kind of layer (will be useful in the future) and add layer config parsing of hasLegend
OL features getId was caught by my IDE as I hadn't enabled case sensitivity... So this reverts all .getId() being replaced by .id on OL features A couple tests also slipped by when renaming baseURL to baseUrl And finally there was a little mistake in how kmlMetadata was handled when loading a legacy KML
939dfa1 to
258f38a
Compare
set default attribution to false across the board, and either fail if none is given (for GeoAdmin layers) or create a default one based on the host for externals So, in essence, attributions are mandatory for GeoAdmin layers, and optional for externals change LayerLegend and GroupOfLayers to also be based on an args constructor
258f38a to
e22a9a9
Compare
ltshb
approved these changes
Mar 4, 2024
ltshb
added a commit
that referenced
this pull request
Mar 4, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
ltshb
added a commit
that referenced
this pull request
Mar 5, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
ltshb
added a commit
that referenced
this pull request
Mar 5, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
ltshb
added a commit
that referenced
this pull request
Mar 5, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
ltshb
added a commit
that referenced
this pull request
Mar 5, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
ltshb
added a commit
that referenced
this pull request
Mar 5, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
ltshb
added a commit
that referenced
this pull request
Mar 6, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
ltshb
added a commit
that referenced
this pull request
Mar 6, 2024
External layer that did not specified its opacity in URL could not be displayed as they default opacity was set to undefined. This has been broken with PR #677
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
2 participants
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.
Main goal : transform layer constructor to have one single arg
side effects :
updateDelayto GeoJSON layersbaseURLis nowbaseUrlserverLayerIdis nottechnicalNamegetID()layer function with attribute.idset/built once in the constructorTest link