Spatial index widgets second proposal #35
Conversation
There was a problem hiding this comment.
I think I prefer computing the spatialFilterResolution internally like this, thanks! This way the user can set the resolution explicitly if the want to, but they don't always need to have the right options to pass into a new function.
I'm not 100% sure about some of the properties moved around on SourceOptionalOptions here, but that's easier to review after merging on the feature branch so I'm not worried about it now.
@juandjara @srtena any other concerns or preferences?
| if (spatialFilter && source.spatialDataType !== 'geo') { | ||
| assert( | ||
| options.spatialFiltersResolution ?? viewState, | ||
| 'spatialFiltersResolution or viewState is required for using spatialFilter with spatial indexes' | ||
| ); | ||
| } | ||
|
|
||
| const spatialFiltersResolution = options.spatialFiltersResolution ?? ( | ||
| getSpatialFiltersResolution({ source, viewState: viewState! }) | ||
| ); |
There was a problem hiding this comment.
Probably we can move the common validation into getSpatialFiltersResolution here, and call the function only if a spatial filter is being used?
There was a problem hiding this comment.
In this case the assert message would need to change to something like
"viewState prop is required to compute automatic spatialFiltersResolution when using spatialFilter with spatial indexes. Either pass a spatialFiltersResolution prop or a viewState prop to avoid this error"
Following up on the conversation here: #34
This is an alternative implementation for the feature of adding widgets to spatial index sources configuring spatial filter to work in that case