PB-101 : zoom on GPX extent on file added #626
Merged
Conversation
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
Passing run #441 ↗︎
Details:
Review all test suite changes for PR #626 ↗︎ |
|||||||||||||||
pakb
force-pushed
the
feat_PB-101_gpx-zoom-to-extent
branch
from
4761db4 to
b3f7679
Compare
pakb
force-pushed
the
feat_PB-101_gpx-zoom-to-extent
branch
2 times, most recently
from
9661d0e to
690237f
Compare
ltshb
reviewed
Jan 31, 2024
| @@ -465,7 +466,7 @@ const actions = { | |||
| if (!extent) { | |||
| updatedLayer.errorKey = 'kml_gpx_file_empty' | |||
| updatedLayer.hasError = true | |||
| } else if (!getKmlExtentForProjection(rootState.position.projection, extent)) { | |||
| } else if (!getExtentForProjection(rootState.position.projection, extent)) { | |||
There was a problem hiding this comment.
I've just seen that in your previous PR for GPX support it seems that you don't support GPX after a reload as the GPX data will not be set (no store subscriber to get the GPX data from url). This means that you would need to add that support and also need a similar logic in the layer store as here and have a updateGpxLayer() and test here again for the extent being in projection bound like for kml. So we would handle the case were an online GPX is updated and the new version is out of bound.
There was a problem hiding this comment.
That's something I wanted to do in another PR
re-using some parts that were made for KML
adding test to better cover getExtentForProjection and getKmlExtent better feedback when a parsed GPX is empty or out of the current projection's bounds
pakb
force-pushed
the
feat_PB-101_gpx-zoom-to-extent
branch
from
690237f to
b0261cb
Compare
ltshb
reviewed
Feb 1, 2024
| // TODO : zoom to extent | ||
| const extent = bbox(gpxToGeoJSON(parseGpx)) | ||
| if (!extent) { | ||
| throw new EmptyGPXError() |
There was a problem hiding this comment.
You also need to handle this error here
pakb
force-pushed
the
feat_PB-101_gpx-zoom-to-extent
branch
from
2f20b59 to
cfa281a
Compare
ltshb
approved these changes
Feb 1, 2024
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.
re-using some parts that were made for KML
Test link