Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: query with geo bounding box #148

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,22 @@ export const DocumentLayerSource = ({
setSelectedLayerConfig({ ...selectedLayerConfig, source });
};

const onToggleGeoBoundingBox = (e: React.ChangeEvent<HTMLInputElement>) => {
const source = { ...selectedLayerConfig.source, useGeoBoundingBoxFilter: e.target.checked };
setSelectedLayerConfig({ ...selectedLayerConfig, source });
};

const shouldTooltipSectionOpen = () => {
return (
selectedLayerConfig.source.showTooltips &&
selectedLayerConfig.source.tooltipFields?.length > 0
);
};

const filterPanelInitialIsOpen =
selectedLayerConfig.source.filters?.length > 0 ||
selectedLayerConfig.source.useGeoBoundingBoxFilter;

return (
<div>
<EuiPanel paddingSize="s">
Expand Down Expand Up @@ -281,7 +290,7 @@ export const DocumentLayerSource = ({
title="Filters"
titleSize="xxs"
isCollapsible={true}
initialIsOpen={selectedLayerConfig.source.filters?.length > 0}
initialIsOpen={filterPanelInitialIsOpen}
>
<SearchBar
appName="maps-dashboards"
Expand All @@ -290,6 +299,17 @@ export const DocumentLayerSource = ({
filters={selectedLayerConfig.source.filters ?? []}
onFiltersUpdated={onFiltersUpdated}
/>
<EuiSpacer />
ruanyl marked this conversation as resolved.
Show resolved Hide resolved
<EuiFormRow>
<EuiCheckbox
id={`${selectedLayerConfig.id}-bounding-box-filter`}
disabled={selectedLayerConfig.source.geoFieldType !== 'geo_point'}
label={'Only request data around map extent'}
checked={selectedLayerConfig.source.useGeoBoundingBoxFilter ? true : false}
onChange={onToggleGeoBoundingBox}
compressed
/>
</EuiFormRow>
</EuiCollapsibleNavGroup>
</EuiPanel>
<EuiSpacer size="m" />
Expand Down
43 changes: 39 additions & 4 deletions maps_dashboards/public/components/map_container/map_container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@

import React, { useEffect, useRef, useState } from 'react';
import { EuiPanel } from '@elastic/eui';
import { LngLat, Map as Maplibre, MapMouseEvent, NavigationControl, Popup } from 'maplibre-gl';
import { LngLat, Map as Maplibre, NavigationControl, Popup, MapEventType } from 'maplibre-gl';
import { debounce } from 'lodash';
import { LayerControlPanel } from '../layer_control_panel';
import './map_container.scss';
import { MAP_INITIAL_STATE, MAP_GLYPHS } from '../../../common';
import { MAP_INITIAL_STATE, MAP_GLYPHS, DASHBOARDS_MAPS_LAYER_TYPE } from '../../../common';
import { MapLayerSpecification } from '../../model/mapLayerType';
import { IndexPattern } from '../../../../../src/plugins/data/public';
import { MapState } from '../../model/mapState';
import { createPopup, getPopupLngLat, isTooltipEnabledLayer } from '../tooltip/create_tooltip';
import { handleDataLayerRender } from '../../model/layerRenderController';
import { useOpenSearchDashboards } from '../../../../../src/plugins/opensearch_dashboards_react/public';
import { MapServices } from '../../types';

interface MapContainerProps {
setLayers: (layers: MapLayerSpecification[]) => void;
Expand All @@ -31,6 +35,7 @@ export const MapContainer = ({
maplibreRef,
mapState,
}: MapContainerProps) => {
const { services } = useOpenSearchDashboards<MapServices>();
const mapContainer = useRef(null);
const [mounted, setMounted] = useState(false);
const [zoom, setZoom] = useState<number>(MAP_INITIAL_STATE.zoom);
Expand Down Expand Up @@ -62,14 +67,15 @@ export const MapContainer = ({
});
}, []);

// Create onClick tooltip for each layer features that has tooltip enabled
useEffect(() => {
let clickPopup: Popup | null = null;
let hoverPopup: Popup | null = null;

// We don't want to show layer information in the popup for the map tile layer
const tooltipEnabledLayers = layers.filter(isTooltipEnabledLayer);

function onClickMap(e: MapMouseEvent) {
function onClickMap(e: MapEventType['click']) {
// remove previous popup
clickPopup?.remove();

Expand All @@ -82,7 +88,7 @@ export const MapContainer = ({
}
}

function onMouseMoveMap(e: MapMouseEvent) {
function onMouseMoveMap(e: MapEventType['mousemove']) {
setCoordinates(e.lngLat.wrap());

// remove previous popup
Expand Down Expand Up @@ -126,6 +132,35 @@ export const MapContainer = ({
};
}, [layers]);

// Handle map bounding box change, it should update the search if "request data around map extent" was enabled
useEffect(() => {
function renderLayers() {
layers.forEach((layer: MapLayerSpecification) => {
// We don't send search query if the layer doesn't have "request data around map extent" enabled
if (
layer.type === DASHBOARDS_MAPS_LAYER_TYPE.DOCUMENTS &&
layer.source.useGeoBoundingBoxFilter
) {
handleDataLayerRender(layer, mapState, services, maplibreRef, undefined);
}
});
}

// Rerender layers with 200ms debounce to avoid calling the search API too frequently, especially when
// resizing the window, the "moveend" event could be fired constantly
const debouncedRenderLayers = debounce(renderLayers, 200);
ruanyl marked this conversation as resolved.
Show resolved Hide resolved

if (maplibreRef.current) {
maplibreRef.current.on('moveend', debouncedRenderLayers);
}

return () => {
if (maplibreRef.current) {
maplibreRef.current.off('moveend', debouncedRenderLayers);
}
};
}, [layers, mapState, services]);

return (
<div>
<EuiPanel hasShadow={false} hasBorder={false} color="transparent" className="zoombar">
Expand Down
6 changes: 5 additions & 1 deletion maps_dashboards/public/components/tooltip/create_tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ type Options = {
export function isTooltipEnabledLayer(
layer: MapLayerSpecification
): layer is DocumentLayerSpecification {
return layer.type !== 'opensearch_vector_tile_map' && layer.source.showTooltips === true;
return (
layer.type !== 'opensearch_vector_tile_map' &&
layer.type !== 'custom_map' &&
layer.source.showTooltips === true
);
}

export function groupFeaturesByLayers(
Expand Down
3 changes: 1 addition & 2 deletions maps_dashboards/public/model/documentLayerFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Map as Maplibre, Popup, MapGeoJSONFeature } from 'maplibre-gl';
import { createPopup, getPopupLngLat } from '../components/tooltip/create_tooltip';
import { Map as Maplibre } from 'maplibre-gl';
import { DocumentLayerSpecification } from './mapLayerType';
import { convertGeoPointToGeoJSON, isGeoJSON } from '../utils/geo_formater';
import { getMaplibreBeforeLayerId, layerExistInMbSource } from './layersFunctions';
Expand Down
59 changes: 55 additions & 4 deletions maps_dashboards/public/model/layerRenderController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
*/

import { Map as Maplibre } from 'maplibre-gl';
import { MapLayerSpecification } from './mapLayerType';
import { DocumentLayerSpecification, MapLayerSpecification } from './mapLayerType';
import { DASHBOARDS_MAPS_LAYER_TYPE } from '../../common';
import {
buildOpenSearchQuery,
Filter,
GeoBoundingBoxFilter,
getTime,
IOpenSearchDashboardsSearchResponse,
isCompleteResponse,
Expand All @@ -20,10 +22,23 @@ interface MaplibreRef {
current: Maplibre | null;
}

// OpenSearch only accepts longitude in range [-180, 180]
// Maplibre could return value out of the range
function adjustLongitudeForSearch(lon: number) {
if (lon < -180) {
return -180;
}
if (lon > 180) {
return 180;
}
return lon;
}

export const prepareDataLayerSource = (
layer: MapLayerSpecification,
mapState: MapState,
{ data, notifications }: MapServices
{ data, notifications }: MapServices,
filters: Filter[] = []
): Promise<any> => {
return new Promise(async (resolve, reject) => {
if (layer.type === DASHBOARDS_MAPS_LAYER_TYPE.DOCUMENTS) {
Expand All @@ -42,6 +57,7 @@ export const prepareDataLayerSource = (
indexPattern,
[],
[
...filters,
...(layer.source.filters ? layer.source.filters : []),
...(timeFilters ? [timeFilters] : []),
]
Expand Down Expand Up @@ -79,13 +95,48 @@ export const prepareDataLayerSource = (
};

export const handleDataLayerRender = (
mapLayer: MapLayerSpecification,
mapLayer: DocumentLayerSpecification,
mapState: MapState,
services: MapServices,
maplibreRef: MaplibreRef,
beforeLayerId: string | undefined
) => {
return prepareDataLayerSource(mapLayer, mapState, services).then((result) => {
const filters: Filter[] = [];
const geoField = mapLayer.source.geoFieldName;
const geoFieldType = mapLayer.source.geoFieldType;

// geo bounding box query supports geo_point fields
if (
geoFieldType === 'geo_point' &&
mapLayer.source.useGeoBoundingBoxFilter &&
maplibreRef.current
) {
const mapBounds = maplibreRef.current.getBounds();
const filterBoundingBox = {
bottom_right: {
lon: adjustLongitudeForSearch(mapBounds.getSouthEast().lng),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will calling mapBounds.getSouthEast().wrap() solves this problem?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VijayanB that won't work out of the box, calling wrap() will wrap the longitude to range (-180, 180]. In the screenshot, after wrapping, the longitude of left bounding is great than the right bounding, the consequence is OpenSearch won't return results from the expected bounding box. What do people usually do to tackle this issue in a map application?

image

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i got your point, let's keep your existing solution for bounds crossing date line. I can ask around, but on top of my head i might consider break it into two polygon and add or filter :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this did we break the query? I hope no because geobounding box query takes care of these things in the code.

Copy link
Member

@VijayanB VijayanB Jan 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@navneet1v i raised new PR to support corner cases here: #175 . Here, i didn't break it into multiple query, instead wrapped the bounds. However, i had put ceiling to MIN/MAX if at least one complete map is visible. ( hybrid approach ) Let me know whether you see any problem with that approach.

lat: mapBounds.getSouthEast().lat,
},
top_left: {
lon: adjustLongitudeForSearch(mapBounds.getNorthWest().lng),
lat: mapBounds.getNorthWest().lat,
},
};
const geoBoundingBoxFilter: GeoBoundingBoxFilter = {
meta: {
disabled: false,
negate: false,
alias: null,
params: filterBoundingBox,
},
geo_bounding_box: {
[geoField]: filterBoundingBox,
},
};
filters.push(geoBoundingBoxFilter);
}

return prepareDataLayerSource(mapLayer, mapState, services, filters).then((result) => {
const { layer, dataSource } = result;
if (layer.type === DASHBOARDS_MAPS_LAYER_TYPE.DOCUMENTS) {
layersFunctionMap[layer.type].render(maplibreRef, layer, dataSource, beforeLayerId);
Expand Down
1 change: 1 addition & 0 deletions maps_dashboards/public/model/mapLayerType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type DocumentLayerSpecification = {
documentRequestNumber: number;
showTooltips: boolean;
tooltipFields: string[];
useGeoBoundingBoxFilter: boolean;
filters: Filter[];
};
style: {
Expand Down