PB-368 : add "load more features" button in feature list#820
Conversation
Passing run #2112 ↗︎
Details:
Review all test suite changes for PR #820 ↗︎ |
|||||||||||||||
49f0630 to
1cfaaee
Compare
|
There is a black background around the tooltip of the zoom to extent, this looks quite ugly |
|
After selecting a bunch of features, clicking outside the selection box where there is nothing to select, the features stay selected when it should clear the selection. |
src/api/features/features.api.js
Outdated
| // there's a hard limit of 50 on our backend | ||
| // see https://api3.geo.admin.ch/services/sdiservices.html#id10 | ||
| limit: Math.min(featureCount, 50), |
There was a problem hiding this comment.
I would not do this here, we might change the backend in future to allow more than 50 and that will make the code hard to understand. Would be better to document the hard limit of 50 in the jsdoc of this function but that's all, if we update the limit to 100, then the backend will just fail and that's fine.
There was a problem hiding this comment.
I'll move this to an app config constant (in src/config.js) it should be then easier to adjust this value later on
src/store/modules/features.store.js
Outdated
| import log from '@/utils/logging' | ||
|
|
||
| const DEFAULT_FEATURE_COUNT_SINGLE_POINT = 10 | ||
| const DEFAULT_FEATURE_COUNT_RECTANGLE_SELECTION = 50 |
There was a problem hiding this comment.
Just wondering if a max of 25 would make more sense and be more performant ? Did you test locally with the prod backend to see if it is faster ? Currently using 50 is quite slow in comparison to the old map geoadmin.
There was a problem hiding this comment.
about 100 to 200ms to get 50 features from the PROD backend, then loading each features' detail takes about 500ms (so in less than 1sec, you get your 50 features rendered on the map).
That's "good enough" but the fact that we need to request each feature's detail adds quite a lot of time to the processing (we could maybe parallelize that in the code)
Best case scenario is to rework what our backend returns so that it is not required to request each feature later, but that's not in the scope of what we want to do here
Seems like it was always there, but having 50 features increase the side-effects of the bug. The tippy selector used in the ZoomToExtentButton was re-applying the same tooltip over and over, with 10 it was still "subtle" shadows, but with 50... well you saw |
8e24df4 to
ddbb91e
Compare
6a5f377 to
8f448df
Compare
Also load 50 features when selecting a rectangle on the map (instead of 10) The piece of code that was triggering the identification of feature was a store plugin, I thought it would make more sense to move all this to the feature store. Especially as I needed to write a second "get more feature" way of loading things. So now the store plugins only triggers a dispatch instead of loading the feature itself.
Also reworking the intercept of feature identification / feature detail, generating random coordinates from a feature template instead of having a very long (and repeating) fixture file. Adapting existing feature selection to use this new way of intercepting/fixtures Fixing a little bug in the randomIntBetween function found while testing (coordinates were not within the bracket when giving something else than 0 as the starting point...)
the selector for the zoom to extent Tippy tooltip was too eager and was re-applying the same tooltip over and over if more than one zoomToExtent button was present at a given time. Adding a component counter to give each component instance a unique ID (and applying Tippy only on this unique ID)
place 10/50 feature limits into config.js and use these new variables in tests (so they adapt to changes) add back "remove selected features" when clicking on a map location without no features
8f448df to
7e29100
Compare
Also load 50 features when selecting a rectangle on the map (instead of 10)
The piece of code that was triggering the identification of feature was a store plugin, I thought it would make more sense to move all this to the feature store. Especially as I needed to write a second "get more feature" way of loading things. So now the store plugins only triggers a dispatch instead of loading the feature itself.
Test link