github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 09:19:04 -04:00
Code Issues Projects Releases Packages Wiki Activity

[maps] fix layer group loading indicator always on when group has non-visible layer (#159517)

Browse source 
closes https://github.com/elastic/kibana/issues/158857

PR resolves issue by adding `isVisible` and `showAtZoomLevel` checks
into `isLayerLoading` and returning false if a layer is not visible (and
therefore, would not load data).

PR also increases test coverage to help ensure regression is not
re-introduced.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Nathan Reese 2023-06-15 09:54:27 -06:00 • committed by GitHub
parent 2b81164ff9
commit 9e60627dd1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 312 additions and 211 deletions
Download patch file Download diff file Expand all files Collapse all files

30
x-pack/plugins/maps/public/classes/layers/ems_vector_tile_layer/ems_vector_tile_layer.test.ts
View file

@ -74,34 +74,4 @@ describe('EmsVectorTileLayer', () => {
expect(layer.getLocale()).toBe('xx');
});
});
describe('isLayerLoading', () => {
test('should be true when tile loading has not started', () => {
const layer = new EmsVectorTileLayer({
source: {} as unknown as EMSTMSSource,
layerDescriptor: {} as unknown as EMSVectorTileLayerDescriptor,
});
expect(layer.isLayerLoading()).toBe(true);
});
test('should be true when tiles are loading', () => {
const layer = new EmsVectorTileLayer({
source: {} as unknown as EMSTMSSource,
layerDescriptor: {
__areTilesLoaded: false,
} as unknown as EMSVectorTileLayerDescriptor,
});
expect(layer.isLayerLoading()).toBe(true);
});
test('should be false when tiles are loaded', () => {
const layer = new EmsVectorTileLayer({
source: {} as unknown as EMSTMSSource,
layerDescriptor: {
__areTilesLoaded: true,
} as unknown as EMSVectorTileLayerDescriptor,
});
expect(layer.isLayerLoading()).toBe(false);
});
});
});

182
x-pack/plugins/maps/public/classes/layers/layer.test.ts
View file

@ -5,27 +5,10 @@
* 2.0.
*/
/* eslint-disable max-classes-per-file */
import { SOURCE_DATA_REQUEST_ID, SOURCE_META_DATA_REQUEST_ID } from '../../../common/constants';
import { AbstractLayer } from './layer';
import { ISource } from '../sources/source';
class MockLayer extends AbstractLayer {}
class MockSource {
private readonly _fitToBounds: boolean;
constructor({ fitToBounds = true } = {}) {
this._fitToBounds = fitToBounds;
}
cloneDescriptor() {
return [{}];
}
async supportsFitToBounds() {
return this._fitToBounds;
}
}
describe('isFittable', () => {
[
{
@ -58,15 +41,164 @@ describe('isFittable', () => {
it(`Should take into account layer visibility and bounds-retrieval: ${JSON.stringify(
test
)}`, async () => {
const layerDescriptor = AbstractLayer.createDescriptor({
visible: test.isVisible,
includeInFitToBounds: test.includeInFitToBounds,
});
const layer = new MockLayer({
layerDescriptor,
source: new MockSource({ fitToBounds: test.fitToBounds }) as unknown as ISource,
const layer = new AbstractLayer({
layerDescriptor: {
visible: test.isVisible,
includeInFitToBounds: test.includeInFitToBounds,
},
source: {
supportsFitToBounds: () => test.fitToBounds,
} as unknown as ISource,
});
expect(await layer.isFittable()).toBe(test.canFit);
});
});
});
describe('isLayerLoading', () => {
test('Should return false when layer is not visible', () => {
const layer = new AbstractLayer({
layerDescriptor: {
visible: false,
},
source: {} as unknown as ISource,
});
expect(layer.isLayerLoading(1)).toBe(false);
});
test('Should return false when layer is not visible at zoom level', () => {
const layer = new AbstractLayer({
layerDescriptor: {
maxZoom: 24,
minZoom: 3,
},
source: {} as unknown as ISource,
});
expect(layer.isLayerLoading(1)).toBe(false);
});
describe('tile layer', () => {
test('Should return true when tile loading has not started', () => {
const layer = new AbstractLayer({
layerDescriptor: {},
source: {} as unknown as ISource,
});
layer._isTiled = () => true;
expect(layer.isLayerLoading(1)).toBe(true);
});
test('Should be true when tiles are loading', () => {
const layer = new AbstractLayer({
layerDescriptor: {
__areTilesLoaded: false,
},
source: {} as unknown as ISource,
});
layer._isTiled = () => true;
expect(layer.isLayerLoading(1)).toBe(true);
});
test('Should be true when tiles are loaded but other data request are pending', () => {
const layer = new AbstractLayer({
layerDescriptor: {
__areTilesLoaded: true,
__dataRequests: [
{
data: {},
dataId: SOURCE_META_DATA_REQUEST_ID,
dataRequestMetaAtStart: {},
dataRequestToken: Symbol(),
},
],
},
source: {} as unknown as ISource,
});
layer._isTiled = () => true;
expect(layer.isLayerLoading(1)).toBe(true);
});
test('Should be false when tiles are loaded and there are no other data requests pending', () => {
const layer = new AbstractLayer({
layerDescriptor: {
__areTilesLoaded: true,
},
source: {} as unknown as ISource,
});
layer._isTiled = () => true;
expect(layer.isLayerLoading(1)).toBe(false);
});
});
describe('non-tile layer', () => {
test('Should return true when source data request has not started', () => {
const layer = new AbstractLayer({
layerDescriptor: {},
source: {} as unknown as ISource,
});
layer._isTiled = () => false;
expect(layer.isLayerLoading(1)).toBe(true);
});
test('Should return true when source data request is pending', () => {
const layer = new AbstractLayer({
layerDescriptor: {
__dataRequests: [
{
data: {},
dataId: SOURCE_DATA_REQUEST_ID,
dataRequestMetaAtStart: {},
dataRequestToken: Symbol(),
},
],
},
source: {} as unknown as ISource,
});
layer._isTiled = () => false;
expect(layer.isLayerLoading(1)).toBe(true);
});
test('Should return true when source data request is finished but other data request are pending', () => {
const layer = new AbstractLayer({
layerDescriptor: {
__dataRequests: [
{
data: {},
dataId: SOURCE_DATA_REQUEST_ID,
dataRequestMeta: {},
dataRequestMetaAtStart: undefined,
dataRequestToken: undefined,
},
{
data: {},
dataId: SOURCE_META_DATA_REQUEST_ID,
dataRequestMetaAtStart: {},
dataRequestToken: Symbol(),
},
],
},
source: {} as unknown as ISource,
});
layer._isTiled = () => false;
expect(layer.isLayerLoading(1)).toBe(true);
});
test('Should return false when source data request is finished and there are no other data requests pending', () => {
const layer = new AbstractLayer({
layerDescriptor: {
__dataRequests: [
{
data: {},
dataId: SOURCE_DATA_REQUEST_ID,
dataRequestMeta: {},
dataRequestMetaAtStart: undefined,
dataRequestToken: undefined,
},
],
},
source: {} as unknown as ISource,
});
layer._isTiled = () => false;
expect(layer.isLayerLoading(1)).toBe(false);
});
});
});

9
x-pack/plugins/maps/public/classes/layers/layer.tsx
View file

@ -67,7 +67,7 @@ export interface ILayer {
getStyleForEditing(): IStyle;
getCurrentStyle(): IStyle;
renderSourceSettingsEditor(sourceEditorArgs: SourceEditorArgs): ReactElement<any> | null;
isLayerLoading(): boolean;
isLayerLoading(zoom: number): boolean;
isFilteredByGlobalTime(): Promise<boolean>;
hasErrors(): boolean;
getErrors(): string;
@ -125,7 +125,7 @@ export type LayerIcon = {
};
export interface ILayerArguments {
layerDescriptor: LayerDescriptor;
layerDescriptor: Partial<LayerDescriptor>;
source: ISource;
}
@ -369,7 +369,10 @@ export class AbstractLayer implements ILayer {
return this._dataRequests.find((dataRequest) => dataRequest.getDataId() === id);
}
isLayerLoading(): boolean {
isLayerLoading(zoom: number): boolean {
if (!this.isVisible() || !this.showAtZoomLevel(zoom)) {
return false;
}
const hasOpenDataRequests = this._dataRequests.some((dataRequest) => dataRequest.isLoading());
if (this._isTiled()) {

77
x-pack/plugins/maps/public/classes/layers/layer_group/layer_group.test.ts
View file

@ -6,17 +6,19 @@
*/
import { MAX_ZOOM, MIN_ZOOM } from '../../../../common/constants';
import { LayerDescriptor } from '../../../../common/descriptor_types';
import { LayerGroup } from './layer_group';
import { ILayer } from '../layer';
import { AbstractLayer, ILayer } from '../layer';
import type { ISource } from '../../sources/source';
describe('getMinZoom', () => {
test('should return MIN_ZOOM when there are no children', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: LayerGroup.createDescriptor({}) });
const layerGroup = new LayerGroup({ layerDescriptor: {} });
expect(layerGroup.getMinZoom()).toBe(MIN_ZOOM);
});
test('should return smallest child.getMinZoom()', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: LayerGroup.createDescriptor({}) });
const layerGroup = new LayerGroup({ layerDescriptor: {} });
layerGroup.setChildren([
{
getMinZoom: () => {
@ -35,12 +37,12 @@ describe('getMinZoom', () => {
describe('getMaxZoom', () => {
test('should return MAX_ZOOM when there are no children', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: LayerGroup.createDescriptor({}) });
const layerGroup = new LayerGroup({ layerDescriptor: {} });
expect(layerGroup.getMaxZoom()).toBe(MAX_ZOOM);
});
test('should return largest child.getMaxZoom()', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: LayerGroup.createDescriptor({}) });
const layerGroup = new LayerGroup({ layerDescriptor: {} });
layerGroup.setChildren([
{
getMaxZoom: () => {
@ -56,3 +58,68 @@ describe('getMaxZoom', () => {
expect(layerGroup.getMaxZoom()).toBe(20);
});
});
describe('isLayerLoading', () => {
function createTiledLayer(layerDescriptor: Partial<LayerDescriptor>) {
const layer = new AbstractLayer({
layerDescriptor,
source: {} as unknown as ISource,
});
layer._isTiled = () => true;
return layer;
}
test('should be true when some children have not started loading', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: {} });
layerGroup.setChildren([
createTiledLayer({
__areTilesLoaded: true,
}),
createTiledLayer({}),
]);
expect(layerGroup.isLayerLoading(1)).toBe(true);
});
test('should be true when some children are loading', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: {} });
layerGroup.setChildren([
createTiledLayer({
__areTilesLoaded: true,
}),
createTiledLayer({
__areTilesLoaded: false,
}),
]);
expect(layerGroup.isLayerLoading(1)).toBe(true);
});
test('should be false when all children have loaded or are not visible', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: {} });
layerGroup.setChildren([
createTiledLayer({
__areTilesLoaded: true,
}),
createTiledLayer({
visible: false,
}),
createTiledLayer({
maxZoom: 24,
minZoom: 3,
}),
]);
expect(layerGroup.isLayerLoading(1)).toBe(false);
});
test('should be false when all children have loaded', async () => {
const layerGroup = new LayerGroup({ layerDescriptor: {} });
layerGroup.setChildren([
createTiledLayer({
__areTilesLoaded: true,
}),
createTiledLayer({
__areTilesLoaded: true,
}),
]);
expect(layerGroup.isLayerLoading(1)).toBe(false);
});
});

6
x-pack/plugins/maps/public/classes/layers/layer_group/layer_group.tsx
View file

@ -58,7 +58,7 @@ export class LayerGroup implements ILayer {
};
}
constructor({ layerDescriptor }: { layerDescriptor: LayerGroupDescriptor }) {
constructor({ layerDescriptor }: { layerDescriptor: Partial<LayerGroupDescriptor> }) {
this._descriptor = LayerGroup.createDescriptor(layerDescriptor);
}
@ -283,9 +283,9 @@ export class LayerGroup implements ILayer {
return undefined;
}
isLayerLoading(): boolean {
isLayerLoading(zoom: number): boolean {
return this._children.some((child) => {
return child.isLayerLoading();
return child.isLayerLoading(zoom);
});
}

10
x-pack/plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_vector_layer.test.tsx
View file

@ -125,7 +125,7 @@ describe('isLayerLoading', () => {
},
} as unknown as IVectorSource,
});
expect(layer.isLayerLoading()).toBe(true);
expect(layer.isLayerLoading(1)).toBe(true);
});
test('should be true when tiles are loading', () => {
@ -144,7 +144,7 @@ describe('isLayerLoading', () => {
},
} as unknown as IVectorSource,
});
expect(layer.isLayerLoading()).toBe(true);
expect(layer.isLayerLoading(1)).toBe(true);
});
test('should be false when tiles are loaded', () => {
@ -163,7 +163,7 @@ describe('isLayerLoading', () => {
},
} as unknown as IVectorSource,
});
expect(layer.isLayerLoading()).toBe(false);
expect(layer.isLayerLoading(1)).toBe(false);
});
test('should be true when tiles are loaded but join is loading', () => {
@ -202,7 +202,7 @@ describe('isLayerLoading', () => {
},
} as unknown as IVectorSource,
});
expect(layer.isLayerLoading()).toBe(true);
expect(layer.isLayerLoading(1)).toBe(true);
});
test('should be false when tiles are loaded and joins are loaded', () => {
@ -243,6 +243,6 @@ describe('isLayerLoading', () => {
},
} as unknown as IVectorSource,
});
expect(layer.isLayerLoading()).toBe(false);
expect(layer.isLayerLoading(1)).toBe(false);
});
});

8
x-pack/plugins/maps/public/classes/layers/vector_layer/vector_layer.tsx
View file

@ -76,7 +76,7 @@ export function isVectorLayer(layer: ILayer) {
export interface VectorLayerArguments {
source: IVectorSource;
joins?: InnerJoin[];
layerDescriptor: VectorLayerDescriptor;
layerDescriptor: Partial<VectorLayerDescriptor>;
customIcons: CustomIcon[];
chartsPaletteServiceGetColor?: (value: string) => string | null;
}
@ -159,7 +159,7 @@ export class AbstractVectorLayer extends AbstractLayer implements IVectorLayer {
this._joins = joins;
this._descriptor = AbstractVectorLayer.createDescriptor(layerDescriptor);
this._style = new VectorStyle(
layerDescriptor.style,
this._descriptor.style,
source,
this,
customIcons,
@ -259,8 +259,8 @@ export class AbstractVectorLayer extends AbstractLayer implements IVectorLayer {
return this.getValidJoins().length > 0;
}
isLayerLoading() {
const isSourceLoading = super.isLayerLoading();
isLayerLoading(zoom: number) {
const isSourceLoading = super.isLayerLoading(zoom);
if (isSourceLoading) {
return true;
}

20
x-pack/plugins/maps/public/connected_components/right_side_controls/layer_control/__snapshots__/layer_control.test.tsx.snap generated
View file

@ -150,25 +150,7 @@ exports[`LayerControl isLayerTOCOpen Should render expand button with error icon
</EuiToolTip>
`;
exports[`LayerControl isLayerTOCOpen spinner icon Should not render expand button with loading icon when layer is invisible 1`] = `
<EuiToolTip
content="Expand layers panel"
delay="long"
display="inlineBlock"
position="left"
>
<EuiButtonIcon
aria-label="Expand layers panel"
className="mapLayerControl__openLayerTOCButton"
color="text"
data-test-subj="mapExpandLayerControlButton"
iconType="menuLeft"
onClick={[Function]}
/>
</EuiToolTip>
`;
exports[`LayerControl isLayerTOCOpen spinner icon Should render expand button with loading icon when layer is loading 1`] = `
exports[`LayerControl isLayerTOCOpen Should render expand button with loading icon when layer is loading 1`] = `
<EuiToolTip
content="Expand layers panel"
delay="long"

62
x-pack/plugins/maps/public/connected_components/right_side_controls/layer_control/layer_control.test.tsx
View file

@ -61,44 +61,24 @@ describe('LayerControl', () => {
expect(component).toMatchSnapshot();
});
describe('spinner icon', () => {
const isLayerLoading = true;
let isVisible = true;
const mockLayerThatIsLoading = {
hasErrors: () => {
return false;
},
isLayerLoading: () => {
return isLayerLoading;
},
isVisible: () => {
return isVisible;
},
showAtZoomLevel: () => {
return true;
},
} as unknown as ILayer;
test('Should render expand button with loading icon when layer is loading', () => {
const component = shallow(
<LayerControl
{...defaultProps}
isLayerTOCOpen={false}
layerList={[mockLayerThatIsLoading]}
/>
);
expect(component).toMatchSnapshot();
});
test('Should not render expand button with loading icon when layer is invisible', () => {
isVisible = false;
const component = shallow(
<LayerControl
{...defaultProps}
isLayerTOCOpen={false}
layerList={[mockLayerThatIsLoading]}
/>
);
expect(component).toMatchSnapshot();
});
test('Should render expand button with loading icon when layer is loading', () => {
const component = shallow(
<LayerControl
{...defaultProps}
isLayerTOCOpen={false}
layerList={[
{
hasErrors: () => {
return false;
},
isLayerLoading: () => {
return true;
},
} as unknown as ILayer,
]}
/>
);
expect(component).toMatchSnapshot();
});
test('Should render expand button with error icon when layer has error', () => {
@ -109,12 +89,6 @@ describe('LayerControl', () => {
isLayerLoading: () => {
return false;
},
isVisible: () => {
return true;
},
showAtZoomLevel: () => {
return true;
},
} as unknown as ILayer;
const component = shallow(
<LayerControl

2
x-pack/plugins/maps/public/connected_components/right_side_controls/layer_control/layer_control.tsx
View file

@ -96,7 +96,7 @@ export function LayerControl({
return layer.hasErrors();
});
const isLoading = layerList.some((layer) => {
return layer.isVisible() && layer.showAtZoomLevel(zoom) && layer.isLayerLoading();
return layer.isLayerLoading(zoom);
});
return (

2
x-pack/plugins/maps/public/connected_components/right_side_controls/layer_control/layer_toc/toc_entry/toc_entry_button/toc_entry_button.tsx
View file

@ -110,7 +110,7 @@ export class TOCEntryButton extends Component<Props, State> {
};
}
if (this.props.layer.isLayerLoading()) {
if (this.props.layer.isLayerLoading(this.props.zoom)) {
return {
icon: <EuiLoadingSpinner size="m" />,
tooltipContent: '',

19
x-pack/plugins/maps/public/routes/map_page/map_app/wait_until_time_layers_load.ts
View file

@ -17,23 +17,20 @@ export function waitUntilTimeLayersLoad$(store: MapStore) {
// using switchMap since switchMap will discard promise from previous state iterations in progress
switchMap(async (state) => {
const zoom = getMapZoom(state);
const promises = getLayerList(state)
.filter((layer) => {
return layer.isVisible() && layer.showAtZoomLevel(zoom);
})
.map(async (layer) => {
return {
isFilteredByGlobalTime: await layer.isFilteredByGlobalTime(),
layer,
};
});
const promises = getLayerList(state).map(async (layer) => {
return {
isFilteredByGlobalTime: await layer.isFilteredByGlobalTime(),
layer,
zoom,
};
});
const layersWithMeta = await Promise.all(promises);
return layersWithMeta;
}),
first((layersWithMeta) => {
const areTimeLayersStillLoading = layersWithMeta
.filter(({ isFilteredByGlobalTime }) => isFilteredByGlobalTime)
.some(({ layer }) => layer.isLayerLoading());
.some(({ layer, zoom }) => layer.isLayerLoading(zoom));
return !areTimeLayersStillLoading;
}),
map(() => {

82
x-pack/plugins/maps/public/selectors/map_selectors.test.ts
View file

@ -151,70 +151,56 @@ describe('getTimeFilters', () => {
});
describe('isMapLoading', () => {
function createLayerMock({
hasErrors = false,
isLayerLoading = false,
isVisible = true,
showAtZoomLevel = true,
}: {
hasErrors?: boolean;
isLayerLoading?: boolean;
isVisible?: boolean;
showAtZoomLevel?: boolean;
}) {
return {
hasErrors: () => {
return hasErrors;
},
isLayerLoading: () => {
return isLayerLoading;
},
isVisible: () => {
return isVisible;
},
showAtZoomLevel: () => {
return showAtZoomLevel;
},
} as unknown as ILayer;
}
test('layers waiting for map to load should not be counted loaded', () => {
test('should return true when there are layers waiting for map to load', () => {
const layerList: ILayer[] = [];
const waitingForMapReadyLayerList: LayerDescriptor[] = [{} as unknown as LayerDescriptor];
const zoom = 4;
expect(isMapLoading.resultFunc(layerList, waitingForMapReadyLayerList, zoom)).toBe(true);
});
test('layer should not be counted as loaded if it has not loaded', () => {
const layerList = [createLayerMock({ isLayerLoading: true })];
test('should return true when there are layers that are loading', () => {
const layerList = [
{
hasErrors: () => {
return false;
},
isLayerLoading: () => {
return true;
},
} as unknown as ILayer,
];
const waitingForMapReadyLayerList: LayerDescriptor[] = [];
const zoom = 4;
expect(isMapLoading.resultFunc(layerList, waitingForMapReadyLayerList, zoom)).toBe(true);
});
test('layer should be counted as loaded if its not visible', () => {
const layerList = [createLayerMock({ isVisible: false, isLayerLoading: true })];
test('should return false when there are unloaded layers with errors', () => {
const layerList = [
{
hasErrors: () => {
return true;
},
isLayerLoading: () => {
return true;
},
} as unknown as ILayer,
];
const waitingForMapReadyLayerList: LayerDescriptor[] = [];
const zoom = 4;
expect(isMapLoading.resultFunc(layerList, waitingForMapReadyLayerList, zoom)).toBe(false);
});
test('layer should be counted as loaded if its not shown at zoom level', () => {
const layerList = [createLayerMock({ showAtZoomLevel: false, isLayerLoading: true })];
const waitingForMapReadyLayerList: LayerDescriptor[] = [];
const zoom = 4;
expect(isMapLoading.resultFunc(layerList, waitingForMapReadyLayerList, zoom)).toBe(false);
});
test('layer should be counted as loaded if it has a loading error', () => {
const layerList = [createLayerMock({ hasErrors: true, isLayerLoading: true })];
const waitingForMapReadyLayerList: LayerDescriptor[] = [];
const zoom = 4;
expect(isMapLoading.resultFunc(layerList, waitingForMapReadyLayerList, zoom)).toBe(false);
});
test('layer should be counted as loaded if its loaded', () => {
const layerList = [createLayerMock({ isLayerLoading: false })];
test('should return false when all layers are loaded', () => {
const layerList = [
{
hasErrors: () => {
return false;
},
isLayerLoading: () => {
return false;
},
} as unknown as ILayer,
];
const waitingForMapReadyLayerList: LayerDescriptor[] = [];
const zoom = 4;
expect(isMapLoading.resultFunc(layerList, waitingForMapReadyLayerList, zoom)).toBe(false);

14
x-pack/plugins/maps/public/selectors/map_selectors.ts
View file

@ -391,12 +391,7 @@ export const isLoadingPreviewLayers = createSelector(
getMapZoom,
(layerList, zoom) => {
return layerList.some((layer) => {
return (
layer.isPreviewLayer() &&
layer.isVisible() &&
layer.showAtZoomLevel(zoom) &&
layer.isLayerLoading()
);
return layer.isPreviewLayer() && layer.isLayerLoading(zoom);
});
}
);
@ -496,12 +491,7 @@ export const isMapLoading = createSelector(
for (let i = 0; i < layerList.length; i++) {
const layer = layerList[i];
if (
layer.isVisible() &&
layer.showAtZoomLevel(zoom) &&
!layer.hasErrors() &&
layer.isLayerLoading()
) {
if (!layer.hasErrors() && layer.isLayerLoading(zoom)) {
return true;
}
}