Skip to content

Commit

Permalink
Better logs and comments
Browse files Browse the repository at this point in the history
Finalizing
Swiper working
  • Loading branch information
Alex-NRCan committed Feb 26, 2024
1 parent 33dc96d commit 455f758
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
}
Expand Down Expand Up @@ -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)?
}

Expand All @@ -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)?
}

Expand All @@ -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)?
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
}
Expand Down Expand Up @@ -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?
}

Expand All @@ -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?
}

Expand All @@ -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

// **********************************************************
Expand Down
54 changes: 18 additions & 36 deletions packages/geoview-core/src/geo/layer/layer.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -400,6 +403,9 @@ export class Layer {
(geoviewLayerConfig) => geoviewLayerConfig.geoviewLayerId !== partialLayerPath
);
}

// Log
logger.logInfo(`Layer removed for ${partialLayerPath}`);
};

/**
Expand Down Expand Up @@ -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<AbstractGeoViewLayer> {
// 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.
Expand Down
61 changes: 40 additions & 21 deletions packages/geoview-core/src/geo/map/map-viewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*/
Expand All @@ -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.
*
Expand All @@ -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'])) {
Expand All @@ -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;
Expand Down Expand Up @@ -219,30 +251,14 @@ 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
*
* @param {string} mapComponentId an id to the new component
* @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));
Expand All @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 10 additions & 1 deletion packages/geoview-swiper/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down
Loading

0 comments on commit 455f758

Please sign in to comment.