[Enhancement] Show an error notification for errors in deck (#1373)

docs/api-reference/actions/actions.md

@@ -961,7 +961,8 @@ It also manges which settings are selected during image and map export
 
 ### addNotification
 
-Add a notification to be displayed
+Add a notification to be displayed.
+Existing notification is going to be updated in case of matching ids.
 
 -   **ActionTypes**: [`ActionTypes.ADD_NOTIFICATION`][12]
 -   **Updaters**: [`uiStateUpdaters.addNotificationUpdater`][220]

docs/api-reference/reducers/ui-state.md

@@ -69,7 +69,8 @@ export default composedReducer;
 
 ### addNotificationUpdater
 
-Add a notification to be displayed
+Add a notification to be displayed.
+Existing notification is going to be updated in case of matching ids.
 
 -   **Action**: [`addNotification`][54]
 

src/actions/ui-state-actions.d.ts

@@ -94,19 +94,18 @@ export function removeNotification(
 export type SetExportImageSettingUpdaterAction = {
   payload: Partial<ExportImage>;
 };
-export function setExportImageSetting(newSetting:
-  SetExportImageSettingUpdaterAction['payload']
+export function setExportImageSetting(
+  newSetting: SetExportImageSettingUpdaterAction['payload']
 ): Merge<SetExportImageSettingUpdaterAction, {type: ActionTypes.SET_EXPORT_IMAGE_SETTING}>;
 
 /** START_EXPORTING_IMAGE */
 export function startExportingImage(options?: {
   ratio?: string;
   resolution?: string;
-  legend?: string,
-  center?: boolean
+  legend?: string;
+  center?: boolean;
 }): Merge<SetExportImageSettingUpdaterAction, {type: ActionTypes.START_EXPORT_IMAGE}>;
 
-
 /** SET_EXPORT_IMAGE_DATA_URI */
 export type SetExportImageDataUriUpdaterAction = {
   payload: string;

src/actions/ui-state-actions.js

@@ -84,9 +84,10 @@ export const toggleMapControl = createAction(ActionTypes.TOGGLE_MAP_CONTROL, (pa
 export const openDeleteModal = createAction(ActionTypes.OPEN_DELETE_MODAL, datasetId => datasetId);
 
 /**
- * Add a notification to be displayed
+ * Add a notification to be displayed.
+ * Existing notification will be updated in case of matching id.
  * @memberof uiStateActions
- * @param notification - The `notification` object to be added
+ * @param notification - The `notification` object to be added or updated
  * @type {typeof import('./ui-state-actions').addNotification}
  * @public
  */

src/components/map-container.js

@@ -25,6 +25,7 @@ import MapboxGLMap from 'react-map-gl';
 import DeckGL from '@deck.gl/react';
 import {createSelector} from 'reselect';
 import WebMercatorViewport from 'viewport-mercator-project';
+import {errorNotification} from 'utils/notifications-utils';
 
 // components
 import MapPopoverFactory from 'components/map/map-popover';
@@ -42,7 +43,11 @@ import {getLayerHoverProp, renderDeckGlLayer} from 'utils/layer-utils';
 
 // default-settings
 import ThreeDBuildingLayer from 'deckgl-layers/3d-building-layer/3d-building-layer';
-import {FILTER_TYPES, GEOCODER_LAYER_ID} from 'constants/default-settings';
+import {
+  FILTER_TYPES,
+  GEOCODER_LAYER_ID,
+  THROTTLE_NOTIFICATION_TIME
+} from 'constants/default-settings';
 
 /** @type {{[key: string]: React.CSSProperties}} */
 const MAP_STYLE = {
@@ -251,6 +256,28 @@ export default function MapContainerFactory(MapPopover, MapControl, Editor) {
       setLayerBlending(gl, this.props.layerBlending);
     };
 
+    _onDeckError = (error, layer) => {
+      const errorMessage = `An error in deck.gl: ${error.message} in ${layer.id}`;
+      const notificationId = `${layer.id}-${error.message}`;
+
+      // Throttle error notifications, as React doesn't like too many state changes from here.
+      this._deckGLErrorsElapsed = this._deckGLErrorsElapsed || {};
+      const lastShown = this._deckGLErrorsElapsed[notificationId];
+      if (!lastShown || lastShown < Date.now() - THROTTLE_NOTIFICATION_TIME) {
+        this._deckGLErrorsElapsed[notificationId] = Date.now();
+
+        // Create new error notification or update existing one with same id.
+        // Update is required to preserve the order of notifications as they probably are going to "jump" based on order of errors.
+        const {uiStateActions} = this.props;
+        uiStateActions.addNotification(
+          errorNotification({
+            message: errorMessage,
+            id: notificationId
+          })
+        );
+      }
+    };
+
     /* component render functions */
 
     /* eslint-disable complexity */
@@ -399,6 +426,7 @@ export default function MapContainerFactory(MapPopover, MapControl, Editor) {
           onBeforeRender={this._onBeforeRender}
           onHover={visStateActions.onLayerHover}
           onClick={visStateActions.onLayerClick}
+          onError={this._onDeckError}
           ref={comp => {
             if (comp && comp.deck && !this._deck) {
               this._deck = comp.deck;

src/constants/action-types.d.ts

@@ -23,7 +23,7 @@ export type ActionType = {
   REMOVE_DATASET: string;
   REORDER_LAYER: string;
   SET_FILTER: string;
-  SET_FILTER_ANIMATION_TIME:  string;
+  SET_FILTER_ANIMATION_TIME: string;
   SET_FILTER_ANIMATION_WINDOW: string;
   SHOW_DATASET_TABLE: string;
   UPDATE_LAYER_BLENDING: string;

src/constants/default-settings.js

@@ -790,6 +790,9 @@ export const DEFAULT_NOTIFICATION_TOPICS = keyMirror({
   file: null
 });
 
+// Minimum time between identical notifications about deck.gl errors
+export const THROTTLE_NOTIFICATION_TIME = 2500;
+
 // Animation
 export const BASE_SPEED = 600;
 export const FPS = 60;

src/reducers/ui-state-updaters.d.ts

@@ -174,9 +174,7 @@ export function setExportMapHTMLModeUpdater(
   state: UiState,
   action: UiStateActions.SetExportHTMLMapModeUpdaterAction
 ): UiState;
-export function showDatasetTableUpdater(
-  state: UiState
-)
+export function showDatasetTableUpdater(state: UiState);
 export function setLocaleUpdater(
   state: UiState,
   action: UiStateActions.SetLocaleUpdaterAction
@@ -186,7 +184,10 @@ export function loadFilesUpdater(state: UiState, action: LoadFilesUpdaterAction)
 export function loadFilesErrUpdater(state: UiState, action: LoadFilesErrUpdaterAction): UiState;
 export function loadFilesSuccessUpdater(state: UiState): UiState;
 export function toggleSplitMapUpdater(state: UiState, action: ToggleSplitMapUpdaterAction): UiState;
-export function initUiStateUpdater(state: UiState, action: {
-  type?: ActionTypes.INIT;
-  payload: KeplerGlInitPayload;
-}): UiState;
+export function initUiStateUpdater(
+  state: UiState,
+  action: {
+    type?: ActionTypes.INIT;
+    payload: KeplerGlInitPayload;
+  }
+): UiState;

src/reducers/ui-state-updaters.js

@@ -596,19 +596,31 @@ export const setExportMapHTMLModeUpdater = (state, {payload: mode}) => ({
 });
 
 /**
- * Add a notification to be displayed
+ * Adds a new notification.
+ * Updates a notification in case of matching ids.
  * @memberof uiStateUpdaters
  * @param state `uiState`
  * @param action
- * @param action.payload
+ * @param action.payload Params of a notification
  * @returns nextState
  * @type {typeof import('./ui-state-updaters').addNotificationUpdater}
  * @public
  */
-export const addNotificationUpdater = (state, {payload}) => ({
-  ...state,
-  notifications: [...(state.notifications || []), createNotification(payload)]
-});
+export const addNotificationUpdater = (state, {payload}) => {
+  let notifications;
+
+  const payloadId = payload?.id;
+  const notificationToUpdate = payloadId ? state.notifications.find(n => n.id === payloadId) : null;
+  if (notificationToUpdate) {
+    notifications = state.notifications.map(n =>
+      n.id === payloadId ? createNotification(payload) : n
+    );
+  } else {
+    notifications = [...(state.notifications || []), createNotification(payload)];
+  }
+
+  return {...state, notifications};
+};
 
 /**
  * Remove a notification

src/utils/notifications-utils.js

@@ -30,7 +30,7 @@ import {
 import {BUG_REPORT_LINK} from 'constants/user-guides';
 
 /**
- * Creates a notofication
+ * Creates a notification
  * @param {object} opt
  * @param {string} opt.message - Message to display during the notification
  * @param {string} opt.type - The type of message. One of DEFAULT_NOTIFICATION_TYPES

test/node/reducers/ui-state-test.js

@@ -29,8 +29,9 @@ import {
   setExportSelectedDataset,
   setExportDataType,
   setExportFiltered,
+  startExportingImage,
   addNotification,
-  startExportingImage
+  removeNotification
 } from 'actions/ui-state-actions';
 import {loadFiles, loadFilesErr} from 'actions/vis-state-actions';
 import {keplerGlInit} from 'actions/actions';
@@ -42,7 +43,6 @@ import {
   DEFAULT_NOTIFICATION_TOPICS,
   DEFAULT_NOTIFICATION_TYPES
 } from 'constants/default-settings';
-import {removeNotification} from 'actions/ui-state-actions';
 
 test('#uiStateReducer', t => {
   t.deepEqual(
@@ -230,25 +230,51 @@ test('#uiStateReducer -> SET_EXPORT_FILTERED', t => {
 });
 
 test('#uiStateReducer -> ADD_NOTIFICATION', t => {
-  const newState = reducer(
-    INITIAL_UI_STATE,
-    addNotification({
-      type: DEFAULT_NOTIFICATION_TYPES.error,
-      message: 'TEST',
-      topic: DEFAULT_NOTIFICATION_TOPICS.global,
-      id: 'test-1'
-    })
+  const sharedNotificationId = 'test-notification-id';
+
+  const notification1 = {
+    type: DEFAULT_NOTIFICATION_TYPES.error,
+    message: 'TEST',
+    topic: DEFAULT_NOTIFICATION_TOPICS.global,
+    id: 'test-1'
+  };
+  const state0 = reducer(INITIAL_UI_STATE, addNotification(notification1));
+  t.equal(state0.notifications.length, 1, 'AddNotification should add one new notification');
+  t.deepEqual(
+    state0.notifications[0],
+    notification1,
+    'AddNotification should have propagated data correctly'
   );
 
-  t.equal(newState.notifications.length, 1, 'AddNotification should add one new notification');
+  const notification2 = {
+    type: DEFAULT_NOTIFICATION_TYPES.info,
+    message: 'TEST',
+    topic: DEFAULT_NOTIFICATION_TOPICS.file,
+    id: sharedNotificationId
+  };
+  const state1 = reducer(state0, addNotification(notification2));
+  t.equal(state1.notifications.length, 2, 'AddNotification should add second notification');
   t.deepEqual(
-    newState.notifications[0],
-    {
-      type: DEFAULT_NOTIFICATION_TYPES.error,
-      message: 'TEST',
-      topic: DEFAULT_NOTIFICATION_TOPICS.global,
-      id: 'test-1'
-    },
+    state1.notifications[1],
+    notification2,
+    'AddNotification should have propagated data correctly '
+  );
+
+  const updatedNotification = {
+    type: DEFAULT_NOTIFICATION_TYPES.error,
+    message: 'TEST-updated-message',
+    topic: DEFAULT_NOTIFICATION_TOPICS.global,
+    id: sharedNotificationId
+  };
+  const state2 = reducer(state1, addNotification(updatedNotification));
+  t.equal(
+    state2.notifications.length,
+    2,
+    "addNotification shouldn't add new notification with same id"
+  );
+  t.deepEqual(
+    state2.notifications[1],
+    updatedNotification,
     'AddNotification should have propagated data correctly '
   );