#2865: Various notification improvements

package.json

@@ -193,7 +193,6 @@
     "@types/json-stringify-safe": "^5.0.0",
     "@types/lodash": "^4.14.171",
     "@types/mustache": "^4.1.2",
-    "@types/notifyjs-browser": "0.0.0",
     "@types/nunjucks": "^3.1.5",
     "@types/object-hash": "^2.1.1",
     "@types/papaparse": "^5.2.6",

src/blocks/renderers/customForm.tsx

@@ -17,13 +17,11 @@
 
 import React from "react";
 import { Renderer } from "@/types";
-import { BlockArg, BlockOptions, ComponentRef, Schema, UiSchema } from "@/core";
+import { BlockArg, ComponentRef, Schema, UiSchema } from "@/core";
 import JsonSchemaForm from "@rjsf/bootstrap-4";
 import { JsonObject } from "type-fest";
 import { dataStore } from "@/background/messenger/api";
-import reportError from "@/telemetry/reportError";
-import { notifyResult } from "@/utils/notify";
-
+import notify from "@/utils/notify";
 import custom from "@/blocks/renderers/customForm.css?loadAsUrl";
 import BootstrapStylesheet from "./BootstrapStylesheet";
 import ImageCropWidget from "@/components/formBuilder/ImageCropWidget";
@@ -99,10 +97,11 @@ export class CustomFormRenderer extends Renderer {
     },
   };
 
-  async render(
-    { recordId, schema, uiSchema }: BlockArg,
-    { logger }: BlockOptions
-  ): Promise<ComponentRef> {
+  async render({
+    recordId,
+    schema,
+    uiSchema,
+  }: BlockArg): Promise<ComponentRef> {
     const formData = await dataStore.get(recordId);
 
     console.debug("Building panel for record: [[ %s ]]", recordId);
@@ -117,19 +116,12 @@ export class CustomFormRenderer extends Renderer {
         async onSubmit(values: JsonObject) {
           try {
             await dataStore.set(recordId, values);
-            notifyResult(logger.context.extensionId, {
-              message: "Saved record",
-              config: {
-                className: "success",
-              },
-            });
+            notify.success("Saved record");
           } catch (error) {
-            reportError(error);
-            notifyResult(logger.context.extensionId, {
+            notify.error({
+              error,
               message: "Error saving record",
-              config: {
-                className: "error",
-              },
+              reportError: false,
             });
           }
         },

src/contentScript/sidebar.tsx

@@ -35,6 +35,7 @@ import pDefer from "p-defer";
 const SIDEBAR_WIDTH_PX = 400;
 const PANEL_CONTAINER_SELECTOR = "#" + PANEL_FRAME_ID;
 export const PANEL_HIDING_EVENT = "pixiebrix:hideSidebar";
+export const SIDEBAR_WIDTH_CSS_PROPERTY = "--pb-sidebar-margin-right";
 
 let renderSequenceNumber = 0;
 
@@ -80,11 +81,16 @@ function storeOriginalCSS() {
 
 function adjustDocumentStyle(): void {
   const $html = getHTMLElement();
-  $html.css("margin-right", `${originalMarginRight + SIDEBAR_WIDTH_PX}px`);
+  $html.css(
+    SIDEBAR_WIDTH_CSS_PROPERTY,
+    `${originalMarginRight + SIDEBAR_WIDTH_PX}px`
+  );
+  $html.css("margin-right", `var(${SIDEBAR_WIDTH_CSS_PROPERTY})`);
 }
 
 function restoreDocumentStyle(): void {
   const $html = getHTMLElement();
+  $html.css(SIDEBAR_WIDTH_CSS_PROPERTY, "");
   $html.css("margin-right", originalMarginRight);
 }
 

src/extensionPoints/menuItemExtension.ts

@@ -17,7 +17,7 @@
 
 import { uuidv4 } from "@/types/helpers";
 import { checkAvailable } from "@/blocks/available";
-import { castArray, cloneDeep, debounce, merge, once } from "lodash";
+import { castArray, cloneDeep, debounce, once } from "lodash";
 import {
   InitialValues,
   reduceExtensionPipeline,
@@ -56,7 +56,7 @@ import { reportEvent } from "@/telemetry/events";
 import notify, {
   DEFAULT_ACTION_RESULTS,
   MessageConfig,
-  notifyResult,
+  showNotification,
 } from "@/utils/notify";
 import { getNavigationId } from "@/contentScript/context";
 import { rejectOnCancelled } from "@/utils";
@@ -557,22 +557,13 @@ export abstract class MenuItemExtensionPoint extends ExtensionPoint<MenuItemExte
 
         extensionLogger.info("Successfully ran menu action");
 
-        notifyResult(
-          extension.id,
-          merge({}, onSuccess, DEFAULT_ACTION_RESULTS.success)
-        );
+        showNotification({ ...DEFAULT_ACTION_RESULTS.success, ...onSuccess });
       } catch (error) {
         if (hasCancelRootCause(error)) {
-          notifyResult(
-            extension.id,
-            merge({}, onCancel, DEFAULT_ACTION_RESULTS.cancel)
-          );
+          showNotification({ ...DEFAULT_ACTION_RESULTS.cancel, ...onCancel });
         } else {
           extensionLogger.error(error);
-          notifyResult(
-            extension.id,
-            merge({}, onError, DEFAULT_ACTION_RESULTS.error)
-          );
+          showNotification({ ...DEFAULT_ACTION_RESULTS.error, ...onError });
         }
       }
     });

src/extensionPoints/quickBarExtension.tsx

@@ -40,7 +40,10 @@ import { castArray, cloneDeep, isEmpty } from "lodash";
 import { checkAvailable, testMatchPatterns } from "@/blocks/available";
 import { BusinessError, hasCancelRootCause } from "@/errors";
 import reportError from "@/telemetry/reportError";
-import notify, { DEFAULT_ACTION_RESULTS, notifyResult } from "@/utils/notify";
+import notify, {
+  DEFAULT_ACTION_RESULTS,
+  showNotification,
+} from "@/utils/notify";
 import { reportEvent } from "@/telemetry/events";
 import { selectEventData } from "@/telemetry/deployments";
 import { selectExtensionContext } from "@/extensionPoints/helpers";
@@ -233,10 +236,10 @@ export abstract class QuickBarExtensionPoint extends ExtensionPoint<QuickBarConf
           });
         } catch (error) {
           if (hasCancelRootCause(error)) {
-            notifyResult(extension.id, DEFAULT_ACTION_RESULTS.cancel);
+            showNotification(DEFAULT_ACTION_RESULTS.cancel);
           } else {
             extensionLogger.error(error);
-            notifyResult(extension.id, DEFAULT_ACTION_RESULTS.error);
+            showNotification(DEFAULT_ACTION_RESULTS.error);
           }
         }
       },

src/utils/notify.module.scss

@@ -1,12 +1,21 @@
+.message {
+  align-self: center;
+}
+
 .closeButton {
   all: initial;
   border: solid 2px #efefff;
   border-radius: 0.4em;
   line-height: 1.4em;
   width: 1.5em;
+
+  flex-shrink: 0;
+  align-self: center;
+
   padding-bottom: 0.1em; // Better vertical alignment
   text-align: center;
   margin-left: 0.5em;
+  margin-right: -10px; // In px because it has to undo/match the component’s px spacing
   cursor: pointer;
 
   &:hover,

src/utils/notify.tsx

@@ -27,6 +27,7 @@ import reportError from "@/telemetry/reportError";
 import { Except, RequireAtLeastOne } from "type-fest";
 import { getErrorMessage } from "@/errors";
 import { truncate } from "lodash";
+import { SIDEBAR_WIDTH_CSS_PROPERTY } from "@/contentScript/sidebar";
 
 const MINIMUM_NOTIFICATION_DURATION = 2000;
 
@@ -71,8 +72,8 @@ const Message: React.VoidFunctionComponent<{
   id: string;
   dismissable: boolean;
 }> = ({ message, id, dismissable }) => (
-  <span>
-    {message}
+  <>
+    <span className={styles.message}>{message}</span>
     {dismissable ? (
       <button
         className={styles.closeButton}
@@ -83,12 +84,12 @@ const Message: React.VoidFunctionComponent<{
         ×
       </button>
     ) : undefined}
-  </span>
+  </>
 );
 
 function getMessageDisplayTime(message: string): number {
   const wpm = 100; // 180 is the average words read per minute, make it slower
-  return Math.min(
+  return Math.max(
     MINIMUM_NOTIFICATION_DURATION,
     (message.split(" ").length / wpm) * 60_000
   );
@@ -115,8 +116,6 @@ export function showNotification({
   /** Only errors are reported by default */
   reportError: willReport = type === "error",
 }: RequireAtLeastOne<Notification, "message" | "error">): string {
-  const options = { id, duration };
-
   if (error) {
     if (!message) {
       message = getErrorMessage(error);
@@ -136,16 +135,24 @@ export function showNotification({
     type = "error";
   }
 
+  const options = {
+    id,
+    duration,
+    // Keep the notification centered on the document even when the sidebar is open
+    style: { marginLeft: `calc(var(${SIDEBAR_WIDTH_CSS_PROPERTY}, 0) * -1)` },
+  };
+  const component = <Message {...{ message, id, dismissable }} />;
+
   switch (type) {
     case "error":
     case "success":
     case "loading":
       // eslint-disable-next-line security/detect-object-injection -- Filtered
-      toast[type](<Message {...{ message, id, dismissable }} />, options);
+      toast[type](component, options);
       break;
 
     default:
-      toast(<Message {...{ message, id, dismissable }} />, options);
+      toast(component, options);
   }
 
   if (willReport) {
@@ -162,38 +169,22 @@ export function hideNotification(id: string): void {
 export const DEFAULT_ACTION_RESULTS = {
   error: {
     message: "Error running action",
-    config: {
-      className: "error",
-    },
+    type: "error",
+    reportError: false,
   },
   cancel: {
     message: "The action was cancelled",
-    config: {
-      className: "info",
-    },
+    type: "info",
   },
   success: {
     message: "Successfully ran action",
-    config: {
-      className: "success",
-    },
+    type: "success",
   },
-};
+} as const;
 
 export interface MessageConfig {
   message: string;
-  config: Partial<NotificationOptions>;
-}
-
-export function notifyResult(
-  extensionId: string,
-  { message, config: { className } }: MessageConfig
-): void {
-  showNotification({
-    message,
-    type: className as NotificationType,
-    reportError: false,
-  });
+  type: NotificationType;
 }
 
 // Private method to prevent adding logic to the `notify.*` helpers.