From 455f758e901f97b8065f0b83f4d2a93804f0219e Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 23 Feb 2024 22:58:14 -0500 Subject: [PATCH] Better logs and comments Finalizing Swiper working --- .../abstract-event-processor.ts | 1 + .../feature-info-event-processor.ts | 2 +- .../geochart-event-processor.ts | 14 +++- .../swiper-event-processor.ts | 34 +++++++- packages/geoview-core/src/geo/layer/layer.ts | 54 +++++-------- .../geoview-core/src/geo/map/map-viewer.ts | 61 ++++++++++----- packages/geoview-swiper/src/index.tsx | 11 ++- packages/geoview-swiper/src/swiper.tsx | 77 +++++++++---------- 8 files changed, 148 insertions(+), 106 deletions(-) diff --git a/packages/geoview-core/src/api/event-processors/abstract-event-processor.ts b/packages/geoview-core/src/api/event-processors/abstract-event-processor.ts index b229b908dd3..23511af8541 100644 --- a/packages/geoview-core/src/api/event-processors/abstract-event-processor.ts +++ b/packages/geoview-core/src/api/event-processors/abstract-event-processor.ts @@ -48,6 +48,7 @@ export abstract class AbstractEventProcessor { // eslint-disable-next-line @typescript-eslint/no-unused-vars protected onInitialize(store: GeoviewStoreType): Array<() => void> | void { + // Here, `store` is unused, but used in inherited classes, so the eslint-disable should be kept // This method should be overriden to initialize and return a list of subscribtions so that they can be destroyed later return undefined; } diff --git a/packages/geoview-core/src/api/event-processors/event-processor-children/feature-info-event-processor.ts b/packages/geoview-core/src/api/event-processors/event-processor-children/feature-info-event-processor.ts index ef9cc9433bb..7035c02f621 100644 --- a/packages/geoview-core/src/api/event-processors/event-processor-children/feature-info-event-processor.ts +++ b/packages/geoview-core/src/api/event-processors/event-processor-children/feature-info-event-processor.ts @@ -57,7 +57,7 @@ export class FeatureInfoEventProcessor extends AbstractEventProcessor { FeatureInfoEventProcessor.deleteFeatureAllInfo(store.getState().mapId, layerPath); // Log - logger.logDebug('Removed Feature Info in stores for layer path:', layerPath); + logger.logInfo('Removed Feature Info in stores for layer path:', layerPath); } }); } diff --git a/packages/geoview-core/src/api/event-processors/event-processor-children/geochart-event-processor.ts b/packages/geoview-core/src/api/event-processors/event-processor-children/geochart-event-processor.ts index 846c0193e8a..07f5cdcc949 100644 --- a/packages/geoview-core/src/api/event-processors/event-processor-children/geochart-event-processor.ts +++ b/packages/geoview-core/src/api/event-processors/event-processor-children/geochart-event-processor.ts @@ -47,9 +47,6 @@ export class GeochartEventProcessor extends AbstractEventProcessor { if (prevOrderedLayerPaths.includes(layerPath) && !curOrderedLayerPaths.includes(layerPath)) { // Remove it GeochartEventProcessor.removeGeochartChart(store.getState().mapId, layerPath); - - // Log - logger.logDebug('Removed GeoChart configs for layer path:', layerPath); } }); } @@ -95,18 +92,23 @@ export class GeochartEventProcessor extends AbstractEventProcessor { const chartData: GeoChartStoreByLayerPath = {}; // Loop on the charts + const layerPaths: string[] = []; charts.forEach((chartInfo) => { // For each layer path chartInfo.layers.forEach((layer) => { // Get the layer path const layerPath = layer.layerId; chartData[layerPath] = chartInfo; + layerPaths.push(layerPath); }); }); // set store charts config this.getGeochartState(mapId)?.actions.setGeochartCharts(chartData); + // Log + logger.logInfo('Added GeoChart configs for layer paths:', layerPaths); + // TODO: Also update the layer array in other store state to inform the later has a geochart attached to it (when code is done over there)? } @@ -128,6 +130,9 @@ export class GeochartEventProcessor extends AbstractEventProcessor { // Update the layer data array in the store this.getGeochartState(mapId)!.actions.setGeochartCharts({ ...this.getGeochartState(mapId)?.geochartChartsConfig, ...toAdd }); + // Log + logger.logInfo('Added GeoChart configs for layer path:', layerPath); + // TODO: Also update the layer array in other store state to inform the later has a geochart attached to it (when code is done over there)? } @@ -153,6 +158,9 @@ export class GeochartEventProcessor extends AbstractEventProcessor { // Update the layer data array in the store this.getGeochartState(mapId)!.actions.setGeochartCharts({ ...chartConfigs }); + // Log + logger.logInfo('Removed GeoChart configs for layer path:', layerPath); + // TODO: Also update the layer array in other store state to inform the later has a geochart attached to it (when code is done over there)? } } diff --git a/packages/geoview-core/src/api/event-processors/event-processor-children/swiper-event-processor.ts b/packages/geoview-core/src/api/event-processors/event-processor-children/swiper-event-processor.ts index bbefbb8eac8..d167de1da15 100644 --- a/packages/geoview-core/src/api/event-processors/event-processor-children/swiper-event-processor.ts +++ b/packages/geoview-core/src/api/event-processors/event-processor-children/swiper-event-processor.ts @@ -41,9 +41,6 @@ export class SwiperEventProcessor extends AbstractEventProcessor { if (prevOrderedLayerPaths.includes(layerPath) && !curOrderedLayerPaths.includes(layerPath)) { // Remove it SwiperEventProcessor.removeLayerPath(store.getState().mapId, layerPath); - - // Log - logger.logDebug('Removed Swiper for layer path:', layerPath); } }); } @@ -74,6 +71,9 @@ export class SwiperEventProcessor extends AbstractEventProcessor { // set store layer paths this.getSwiperState(mapId)?.actions.setLayerPaths(layerPaths); + // Log + logger.logInfo('Added Swiper functionality for layer paths:', layerPaths); + // TODO: Also update the layer array in other store state to inform the later has a swiper attached to it? } @@ -95,6 +95,9 @@ export class SwiperEventProcessor extends AbstractEventProcessor { // Update the layer data array in the store this.getSwiperState(mapId)!.actions.setLayerPaths(updatedArray); + // Log + logger.logInfo('Added Swiper functionality for layer path:', layerPath); + // TODO: Also update the layer array in other store state to inform the later has a swiper attached to it? } @@ -121,10 +124,35 @@ export class SwiperEventProcessor extends AbstractEventProcessor { // Update the layer data array in the store this.getSwiperState(mapId)!.actions.setLayerPaths(updatedArray); + // Log + logger.logInfo('Removed Swiper functionality for layer path:', layerPath); + // TODO: Also update the layer array in other store state to inform the later has a swiper attached to it? } } + /** + * Removes the swipe functionality for all layer paths + * @param {string} mapId The map ID + */ + static removeAll(mapId: string) { + // The processor needs an initialized layer paths store which is only initialized if the Swiper Plugin exists. + // Therefore, we validate its existence first. + if (!this.getSwiperState(mapId)) return; + if (!this.getSwiperState(mapId)?.layerPaths) return; + + // Get all layer paths + const { layerPaths } = this.getSwiperState(mapId)!; + + // Update the layer data array in the store + this.getSwiperState(mapId)!.actions.setLayerPaths([]); + + // Log + logger.logInfo('Removed Swiper functionality for all layer paths', layerPaths); + + // TODO: Also update the layer array in other store state to inform the later has a swiper attached to it? + } + // #endregion // ********************************************************** diff --git a/packages/geoview-core/src/geo/layer/layer.ts b/packages/geoview-core/src/geo/layer/layer.ts index f6fd404d535..639d0d1b4b5 100644 --- a/packages/geoview-core/src/geo/layer/layer.ts +++ b/packages/geoview-core/src/geo/layer/layer.ts @@ -1,3 +1,5 @@ +import BaseLayer from 'ol/layer/Base'; +import LayerGroup from 'ol/layer/Group'; import { GeoCore, layerConfigIsGeoCore } from '@/geo/layer/other/geocore'; import { Geometry } from '@/geo/layer/geometry/geometry'; import { FeatureHighlight } from '@/geo/utils/feature-highlight'; @@ -311,15 +313,16 @@ export class Layer { * @returns {AbstractGeoViewLayer} Returns the geoview instance associated to the layer path. */ geoviewLayer(layerPath: string): AbstractGeoViewLayer { - // TODO: Refactor - Move this method next to the getGeoviewLayerByLayerPath equivalent. And rename it? - // The first element of the layerPath is the geoviewLayerId + // TODO: Refactor - Move this method next to the getGeoviewLayerByLayerPath equivalent. And then rename it? + // The first element of the layerPath is the geoviewLayerId const geoviewLayerInstance = this.geoviewLayers[layerPath.split('/')[0]]; - // TODO: Check - Why set the `layerPathAssociatedToTheGeoviewLayer` property on the fly like that? Should likely set this somewhere else than a function that looks like a getter. + // TODO: Check #1857 - Why set the `layerPathAssociatedToTheGeoviewLayer` property on the fly like that? Should likely set this somewhere else than in this function that looks more like a getter. // TO.DOCONT: It seems `layerPathAssociatedToTheGeoviewLayer` is indeed used many places, notably in applyFilters logic. - // TO.DOCONT: If all those places rely on the `layerPathAssociatedToTheGeoviewLayer` to be set, that logic should be moved over there. - // TO.DOCONT: If there's more other places relying on the `layerPathAssociatedToTheGeoviewLayer`, this property set assumes/relies on the fact that this geoviewLayer() was called. - // TO.DOCONT: There's likely some separation of logic to apply here and/or make this function more evident that it 'sets' something, not just 'gets' a GeoViewLayer. + // TO.DOCONT: If all those places rely on the `layerPathAssociatedToTheGeoviewLayer` to be set, that logic using layerPathAssociatedToTheGeoviewLayer should be moved over there. + // TO.DOCONT: If there's more other places relying on the `layerPathAssociatedToTheGeoviewLayer`, then it's not ideal, + // TO.DOCONT: because it's assuming/relying on the fact that all those other places use this specific geoviewLayer() prior to do their work. + // TO.DOCONT: There's likely some separation of logic to apply here. Make this function more evident that it 'sets' something, not just 'gets' a GeoViewLayer. geoviewLayerInstance.layerPathAssociatedToTheGeoviewLayer = layerPath; return geoviewLayerInstance; } @@ -400,6 +403,9 @@ export class Layer { (geoviewLayerConfig) => geoviewLayerConfig.geoviewLayerId !== partialLayerPath ); } + + // Log + logger.logInfo(`Layer removed for ${partialLayerPath}`); }; /** @@ -515,39 +521,15 @@ export class Layer { }; /** - * Returns the GeoView instance associated to a specific layer path. The first element of the layerPath - * is the geoviewLayerId. - * @param {string} layerPath The layer path to the layer's configuration. - * - * @returns {AbstractGeoViewLayer} Returns the geoview instance associated to the layer path. - */ - getGeoviewLayerByLayerPath(layerPath: string): AbstractGeoViewLayer | null { - // The first element of the layerPath is the geoviewLayerId - const geoviewLayerId = layerPath.split('/')[0]; - - // Redirect - return this.getGeoviewLayerById(geoviewLayerId); - } - - /** - * Returns the GeoView instance associated to a specific layer path. The first element of the layerPath - * is the geoviewLayerId. + * Returns the OpenLayer layer associated to a specific layer path. * @param {string} layerPath The layer path to the layer's configuration. * - * @returns {AbstractGeoViewLayer} Returns the geoview instance associated to the layer path. + * @returns {BaseLayer | LayerGroup | null} Returns the OpenLayer layer associated to the layer path. */ - getGeoviewLayerByLayerPathAsync( - layerPath: string, - mustBeProcessed: boolean, - timeout?: number, - checkFrequency?: number - ): Promise { - // The first element of the layerPath is the geoviewLayerId - const geoviewLayerId = layerPath.split('/')[0]; - - // Redirect - return this.getGeoviewLayerByIdAsync(geoviewLayerId, mustBeProcessed, timeout, checkFrequency); - } + getLayerByLayerPath = (layerPath: string): BaseLayer | LayerGroup | null => { + // Return the olLayer object from the registered layers + return api.maps[this.mapId].layer.registeredLayers[layerPath]?.olLayer; + }; /** * Returns a Promise that will be resolved once the given layer is in a processed phase. diff --git a/packages/geoview-core/src/geo/map/map-viewer.ts b/packages/geoview-core/src/geo/map/map-viewer.ts index 8d6ccd8b7f8..7c0e9e1acec 100644 --- a/packages/geoview-core/src/geo/map/map-viewer.ts +++ b/packages/geoview-core/src/geo/map/map-viewer.ts @@ -56,6 +56,7 @@ import { MapEventProcessor } from '@/api/event-processors/event-processor-childr import { AppEventProcessor } from '@/api/event-processors/event-processor-children/app-event-processor'; import { logger } from '@/core/utils/logger'; +// TODO: Refactor - Typo TypeDcoument - actually, remove the type altogether, doesn't seem useful interface TypeDcoument extends Document { webkitExitFullscreen: () => void; msExitFullscreen: () => void; @@ -115,8 +116,15 @@ export class MapViewer { private i18nInstance!: i18n; /** - * Add the map instance to the maps array in the api - * + * Constructor for a MapViewer, setting: + * - the mapId + * - the mapFeaturesConfig + * - i18n + * - appbar, navbar, footerbar + * - modalApi + * - geoviewRenderer + * - basemap + * - layers * @param {TypeMapFeaturesConfig} mapFeaturesConfig map properties * @param {i18n} i18instance language instance */ @@ -142,6 +150,26 @@ export class MapViewer { this.setLayerAddedListener4ThisListOfLayer(listOfGeoviewLayerConfig); } + /** + * Initializes map, layer class and geometries + * + * @param {OLMap} cgpMap The OpenLayers map object + */ + initMap(cgpMap: OLMap): void { + // Set the map + this.map = cgpMap; + + // TODO: Refactor - Is it necessary to set the mapId again? It was set in constructor. Preferably set it at one or the other. + this.mapId = cgpMap.get('mapId'); + + // initialize layers and load the layers passed in from map config if any + this.layer = new Layer(this.mapId); + this.layer.loadListOfGeoviewLayer(this.mapFeaturesConfig.map.listOfGeoviewLayerConfig); + + // check if geometries are provided from url + this.loadGeometries(); + } + /** * Set the layer added event listener and timeout function for the list of geoview layer configurations. * @@ -159,6 +187,10 @@ export class MapViewer { if (payloadIsGeoViewLayerAdded(payload)) { const { geoviewLayer } = payload; + + // Log + logger.logInfo(`GeoView Layer added on map ${geoviewLayer.geoviewLayerId}`, geoviewLayer); + MapEventProcessor.setLayerZIndices(this.mapId); // If metadata are processed if (geoviewLayer.allLayerStatusAreIn(['processed', 'loading', 'loaded', 'error'])) { @@ -174,10 +206,10 @@ export class MapViewer { } /** - * Method used to test all geoview layers status flags to determine if all the metadata of the map are loaded. + * Tests all geoview layers status flags to determine if all the metadata of the map are loaded. * This doesn't mean that all the layers are loaded on the map, Only the metadata are read and processed. * - * @returns true if all geoview layers on the map are loaded or detected as a load error. + * @returns {true} if all geoview layers on the map are loaded or detected as a load error. */ mapIsReady(): boolean { if (this.layer === undefined) return false; @@ -219,23 +251,6 @@ export class MapViewer { }, 250); } - /** - * Initialize layers, basemap and projection - * - * @param cgpMap - */ - initMap(cgpMap: OLMap): void { - this.mapId = cgpMap.get('mapId'); - this.map = cgpMap; - - // initialize layers and load the layers passed in from map config if any - this.layer = new Layer(this.mapId); - this.layer.loadListOfGeoviewLayer(this.mapFeaturesConfig.map.listOfGeoviewLayerConfig); - - // check if geometries are provided from url - this.loadGeometries(); - } - /** * Add a new custom component to the map * @@ -243,6 +258,7 @@ export class MapViewer { * @param {JSX.Element} component the component to add */ addComponent(mapComponentId: string, component: JSX.Element): void { + // TODO: Refactor - Doesn't seem to be used anymore, keep? If keep it, refactor to call a function instead of event.emit. if (mapComponentId && component) { // emit an event to add the component api.event.emit(mapComponentPayload(EVENT_NAMES.MAP.EVENT_MAP_ADD_COMPONENT, this.mapId, mapComponentId, component)); @@ -255,6 +271,7 @@ export class MapViewer { * @param imapComponentIdd the id of the component to remove */ removeComponent(mapComponentId: string): void { + // TODO: Refactor - Doesn't seem to be used anymore, keep? If keep it, refactor to call a function instead of event.emit. if (mapComponentId) { // emit an event to add the component api.event.emit(mapComponentPayload(EVENT_NAMES.MAP.EVENT_MAP_REMOVE_COMPONENT, this.mapId, mapComponentId)); @@ -318,6 +335,8 @@ export class MapViewer { * @param {HTMLElement} element the element to toggle fullscreen on */ setFullscreen(status: boolean, element: TypeHTMLElement): void { + // TODO: Refactor - For reusability, this function should be static and moved to a browser-utilities class + // TO.DOCONT: If we want to keep a function here, in MapViewer, it should just be a redirect to the browser-utilities' // enter fullscreen if (status) { if (element.requestFullscreen) { diff --git a/packages/geoview-swiper/src/index.tsx b/packages/geoview-swiper/src/index.tsx index 6dba6f4391d..006fc1678eb 100644 --- a/packages/geoview-swiper/src/index.tsx +++ b/packages/geoview-swiper/src/index.tsx @@ -68,7 +68,7 @@ class SwiperPlugin extends MapPlugin { */ activateForLayer = (layerPath: string) => { // Check if the layer exists on the map - const layer = this.map().layer.getGeoviewLayerByLayerPath(layerPath); + const layer = this.map().layer.getLayerByLayerPath(layerPath); if (layer) { // Add the layer path SwiperEventProcessor.addLayerPath(this.pluginProps.mapId, layerPath); @@ -86,6 +86,15 @@ class SwiperPlugin extends MapPlugin { // Remove the layer SwiperEventProcessor.removeLayerPath(this.pluginProps.mapId, layerPath); }; + + /** + * Deactivates the swiper for the layer indicated by the given layer path. + * @param {string} layerPath The layer path to deactivate swiper functionality + */ + deActivateAll = () => { + // Remove all layers + SwiperEventProcessor.removeAll(this.pluginProps.mapId); + }; } export default SwiperPlugin; diff --git a/packages/geoview-swiper/src/swiper.tsx b/packages/geoview-swiper/src/swiper.tsx index 7760a387ead..e3b114f3596 100644 --- a/packages/geoview-swiper/src/swiper.tsx +++ b/packages/geoview-swiper/src/swiper.tsx @@ -1,6 +1,7 @@ import Draggable from 'react-draggable'; import { RefObject } from 'geoview-core'; +import { MapViewer } from 'geoview-core/src/geo/map/map-viewer'; import { useSwiperLayerPaths } from 'geoview-core/src/core/stores/store-interface-and-intial-values/swiper-state'; import { getLocalizedMessage } from 'geoview-core/src/core/utils/utilities'; import { logger } from 'geoview-core/src/core/utils/logger'; @@ -33,8 +34,9 @@ export function Swiper(props: SwiperProps): JSX.Element { const { useEffect, useState, useRef, useCallback } = react; const { Box, Tooltip, HandleIcon } = ui.elements; const { orientation } = config; + const mapViewer = api.maps[mapId] as MapViewer; - const mapSize = useRef(api.maps[mapId].map?.getSize() || [0, 0]); + const mapSize = useRef(mapViewer.map?.getSize() || [0, 0]); const swiperValue = useRef(50); const swiperRef = useRef(); @@ -51,6 +53,9 @@ export function Swiper(props: SwiperProps): JSX.Element { */ const prerender = useCallback( (event: Event | BaseEvent) => { + // Log + logger.logTraceUseCallback('GEOVIEW-SWIPER - prerender', event); + const evt = event as RenderEvent; const ctx: CanvasRenderingContext2D = evt.context! as CanvasRenderingContext2D; const width = ((mapSize.current[0] + 6) * swiperValue.current) / 100; @@ -113,7 +118,7 @@ export function Swiper(props: SwiperProps): JSX.Element { */ const onStop = debounce(() => { // get map size - mapSize.current = api.maps[mapId].map.getSize() || [0, 0]; + mapSize.current = mapViewer.map.getSize() || [0, 0]; const size = orientation === 'vertical' ? mapSize.current[0] : mapSize.current[1]; const position = orientation === 'vertical' ? getSwiperStyle()[0] : getSwiperStyle()[1]; swiperValue.current = (position / size) * 100; @@ -155,41 +160,30 @@ export function Swiper(props: SwiperProps): JSX.Element { } }, 100); - /** - * Retrieves a GeoviewLayer object given the layer path and wires `precompose` and `postcompose` events to - * support swiping rendering on the map. - * @param {string} layerPath The layer path for the layer to wire rendering events - */ - const attachEventWhenReady = useCallback( - async (layerPath: string) => { + useEffect(() => { + // Log + logger.logTraceUseEffect('GEOVIEW-SWIPER - layerPaths', layerPaths); + + // Set listener for layers in config array + layerPaths.forEach((layerPath: string) => { try { - // Get the layer by id once the layer is in a 'processed' state - const gvLayer = await api.maps[mapId].layer.getGeoviewLayerByLayerPathAsync(layerPath, true); - const olLayer = gvLayer.olLayers!; - setOlLayers((prevArray: BaseLayer[]) => [...prevArray, olLayer!]); + // Get the layer at the layer path + const olLayer = mapViewer.layer.getLayerByLayerPath(layerPath); + if (olLayer) { + // Set the OL layers + setOlLayers((prevArray: BaseLayer[]) => [...prevArray, olLayer]); - // Wire events on the layer - olLayer?.on(['precompose' as EventTypes, 'prerender' as EventTypes], prerender); - olLayer?.on(['postcompose' as EventTypes, 'postrender' as EventTypes], postcompose); + // Wire events on the layer + olLayer.on(['precompose' as EventTypes, 'prerender' as EventTypes], prerender); + olLayer.on(['postcompose' as EventTypes, 'postrender' as EventTypes], postcompose); - // Force refresh - olLayer?.changed(); + // Force refresh + olLayer.changed(); + } } catch (error) { // Log - logger.logError('SWIPER - Failed to attach layer events', api.maps[mapId].layer?.geoviewLayers, layerPath, error); + logger.logError('SWIPER - Failed to attach layer events', mapViewer.layer?.geoviewLayers, layerPath, error); } - }, - [api.maps, mapId, prerender] - ); - - useEffect(() => { - // Log - logger.logTraceUseEffect('GEOVIEW-SWIPER - layerPaths', layerPaths); - - // Set listener for layers in config array - layerPaths.forEach((layer: string) => { - // Attach event when layer will be ready - attachEventWhenReady(layer); }); return () => { @@ -199,15 +193,16 @@ export function Swiper(props: SwiperProps): JSX.Element { // set listener for layers in config array layerPaths.forEach((layerPath: string) => { try { - const gvLayer = api.maps[mapId].layer.getGeoviewLayerByLayerPath(layerPath); - const olLayer = gvLayer.olLayers!; - - // Unwire the events on the layer - olLayer?.un(['precompose' as EventTypes, 'prerender' as EventTypes], prerender); - olLayer?.un(['postcompose' as EventTypes, 'postrender' as EventTypes], postcompose); - - // Force refresh - olLayer?.changed(); + // Get the layer at the layer path + const olLayer = mapViewer.layer.getLayerByLayerPath(layerPath); + if (olLayer) { + // Unwire the events on the layer + olLayer.un(['precompose' as EventTypes, 'prerender' as EventTypes], prerender); + olLayer.un(['postcompose' as EventTypes, 'postrender' as EventTypes], postcompose); + + // Force refresh + olLayer.changed(); + } } catch (error) { // Log logger.logError('SWIPER - Failed to un-attach layer events', layerPath, error); @@ -217,7 +212,7 @@ export function Swiper(props: SwiperProps): JSX.Element { // Empty layers array setOlLayers([]); }; - }, [api.maps, mapId, layerPaths, attachEventWhenReady, prerender]); + }, [mapViewer, layerPaths, prerender]); useEffect(() => { // Log