Skip to content

Commit

Permalink
Merge branch 'main' into F/ci/webext-alert
Browse files Browse the repository at this point in the history
  • Loading branch information
fregante authored Jan 22, 2024
2 parents 0ceeb4e + 4716134 commit 155223e
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 18 deletions.
4 changes: 2 additions & 2 deletions src/bricks/effects/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ export class AlertEffect extends EffectABC {
const messageString = String(message);

if (type === "window") {
// eslint-disable-next-line no-alert
// eslint-disable-next-line no-alert -- User requested
window.alert(messageString);
} else {
showNotification({
message: messageString,
type,
duration,
autoDismissTimeMs: duration,
reportError: false,
});
}
Expand Down
18 changes: 10 additions & 8 deletions src/errors/contextInvalidated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,29 @@ import { getErrorMessage, getRootCause } from "./errorHelpers";
import { CONTEXT_INVALIDATED_ERROR } from "@/errors/knownErrorMessages";

/**
* Notification id to avoid displaying the same notification multiple times.
* Notification id to avoid displaying multiple notifications at once.
*/
const CONNECT_LOST_NOTIFICATION_ID = "connection-lost";
const CONTEXT_INVALIDATED_NOTIFICATION_ID = "context-invalidated";

const CONTEXT_INVALIDATED_NOTIFICATION_DURATION_MS = 20_000;

/**
* Display a notification when the background page unloads/reloads because at this point
* all communication becomes impossible.
*/
export async function notifyContextInvalidated(): Promise<void> {
// `import()` is needed to avoid execution of its dependencies (telemetry)
// Also, lazily import to avoid importing React unnecessarily
// Lazily import React component. Also avoids a `webext-messenger` transitive dependency.
// https://github.com/pixiebrix/pixiebrix-extension/pull/6234
// https://github.com/pixiebrix/pixiebrix-extension/issues/4058#issuecomment-1217391772
// eslint-disable-next-line import/dynamic-import-chunkname
const { default: notify } = await import(
/* webpackMode: "lazy" */ "@/utils/notify"
/* webpackChunkName: "notify" */ "@/utils/notify"
);

notify.error({
id: CONNECT_LOST_NOTIFICATION_ID,
id: CONTEXT_INVALIDATED_NOTIFICATION_ID,
message: "PixieBrix was updated or restarted. Reload the page to continue",
reportError: false, // It cannot report it because its background page no longer exists
duration: Number.POSITIVE_INFINITY,
autoDismissTimeMs: CONTEXT_INVALIDATED_NOTIFICATION_DURATION_MS,
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/extensionConsole/pages/settings/AdvancedSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ const AdvancedSettings: React.FunctionComponent = () => {
id: SAVING_URL_NOTIFICATION_ID,
message: "Service URL updated. The extension must be reloaded",
dismissable: false,
duration: Number.POSITIVE_INFINITY,
autoDismissTimeMs: Number.POSITIVE_INFINITY,
});
},
[serviceURL, setServiceURL],
Expand Down
2 changes: 1 addition & 1 deletion src/utils/clipboardUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ async function interactiveWriteToClipboard(

const notificationId = notify.info({
message: `Click anywhere to copy ${type} to clipboard`,
duration: Number.POSITIVE_INFINITY,
autoDismissTimeMs: Number.POSITIVE_INFINITY,
dismissable: false,
});

Expand Down
12 changes: 6 additions & 6 deletions src/utils/notify.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { SIDEBAR_WIDTH_CSS_PROPERTY } from "@/contentScript/sidebarDomController
import ErrorIcon from "@/icons/error.svg?loadAsComponent";
import WarningIcon from "@/icons/warning.svg?loadAsComponent";

const MINIMUM_NOTIFICATION_DURATION = 2000;
const MINIMUM_NOTIFICATION_DURATION_MS = 2000;

export type NotificationType =
| "info"
Expand All @@ -51,7 +51,7 @@ type Notification = RequireAtLeastOne<
message: string;
type?: NotificationType;
id?: string;
duration?: number;
autoDismissTimeMs?: number;
error: unknown;
dismissable?: boolean;
reportError?: boolean;
Expand Down Expand Up @@ -114,10 +114,10 @@ const toastStyle: ToastStyle = {
},
};

function getMessageDisplayTime(message: string): number {
function getMessageDisplayTimeMs(message: string): number {
const wpm = 100; // 180 is the average words read per minute, make it slower
return Math.max(
MINIMUM_NOTIFICATION_DURATION,
MINIMUM_NOTIFICATION_DURATION_MS,
(message.split(" ").length / wpm) * 60_000,
);
}
Expand Down Expand Up @@ -149,7 +149,7 @@ export function showNotification({
message,
type = error ? "error" : undefined,
id = uuidv4(),
duration,
autoDismissTimeMs: duration,
dismissable = true,

/** Only errors are reported by default */
Expand All @@ -166,7 +166,7 @@ export function showNotification({
// Avoid excessively-long notification messages
message = truncate(message, { length: 400 });

duration ??= getMessageDisplayTime(message);
duration ??= getMessageDisplayTimeMs(message);

const options: ToastOptions = {
id,
Expand Down

0 comments on commit 155223e

Please sign in to comment.