PB-95: Add zoom to KML extent in import file #608
Conversation
Passing run #337 ↗︎
Details:
Review all test suite changes for PR #608 ↗︎ |
|||||||||||||||
ltshb
force-pushed
the
feat-PB-95-kml-zoom-to-extent
branch
2 times, most recently
from
584a43e to
dea5181
Compare
ltshb
force-pushed
the
feat-PB-95-kml-zoom-to-extent
branch
5 times, most recently
from
e2fc458 to
5da960c
Compare
|
@pakb I don't understand why the cesium test is now failing ? The comment about empty collection seems also a bit strange Do you know if the test is really correct ? |
|
Hey, @ltshb Since the failing test checks the drawings displayed once in 3d mode, my guess is it doesn't see the layers that haven't been loaded correctly. |
ltshb
force-pushed
the
feat-PB-95-kml-zoom-to-extent
branch
from
5da960c to
576f945
Compare
|
@ltkum thanks for the hint I found the bug. |
ltshb
force-pushed
the
feat-PB-95-kml-zoom-to-extent
branch
2 times, most recently
from
750b58a to
4050cfd
Compare
ltshb
force-pushed
the
feat-PB-95-kml-zoom-to-extent
branch
from
4050cfd to
c2b056c
Compare
| setLayerErrorKey({ commit, getters }, payload) { | ||
| const { layerId, errorKey } = payload | ||
| const currentLayer = getters.getActiveLayerById(layerId) | ||
| if (!currentLayer) { | ||
| throw new Error( | ||
| `Failed to update layer error key "${layerId}", layer not found in active layers` | ||
| ) | ||
| } | ||
| const updatedLayer = currentLayer.clone() | ||
| updatedLayer.errorKey = errorKey | ||
| updatedLayer.hasError = !!errorKey | ||
| commit('updateLayer', updatedLayer) | ||
| }, |
There was a problem hiding this comment.
Shouldn't that only be applied to KML layers?
If that's not what is intended, I would move the declaration of errorKey and hasError into AbstractLayer.class.js instead of KMLLayer.class.js
There was a problem hiding this comment.
errorKey and hasError were meant for ExternalLayer but because for KML we have only one class for internal and external I had to put the flag again in this class. But this is a good idea to move them into the abstract layer even if for the moment they are only used for external layer (internal layer should never have an error ;-) ).
There was a problem hiding this comment.
a couple unit tests to cover these two new utils wouldn't hurt (if their usage isn't limited to KML utils only)
As they stand now, they can be used elsewhere, so I would cover them with unit test (the basic, input checking and such cases)
Those flage are only intended for external layer but because not all external layer extends ExternalLayer (e.g. KML) I moved those flag into the abstract class.
ltshb
force-pushed
the
feat-PB-95-kml-zoom-to-extent
branch
from
8351f02 to
1428a01
Compare
Now when importing a KML file we automatically zoom to its extent
Test link