Minor improvements

Author: andreas-schultz

CHANGELOG.md

@@ -9,13 +9,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 ### Added
 
 -   `<ContextOverlay />`
-    -   `usePlaceholder` property, can be used to display the target but include the component later when the first interaction happens, this can improve performance
+    -   `usePlaceholder` property: can be used to display the target but include the component later when the first interaction happens, this can improve performance
 -   `<ContextMenu />`
-    -   `preventPlaceholder` property, to prevent the default usage of placeholders waiting for the first user interaction before inserting the full context menu
+    -   `preventPlaceholder` property to prevent the default usage of placeholders waiting for the first user interaction before inserting the full context menu
 -   `<Tooltip />`
-    -   `usePlaceholder` property, can be used to display the target but include the full component later when the first interaction happens, this can improve performance. It is turned on for text tooltips by default.
+    -   `usePlaceholder` property: can be used to display the target but include the full component later when the first interaction happens, this can improve performance. It is turned on for text tooltips by default.
 -   `<OverviewItemActions />`
-    -   `delayDisplayChildren` property, set a time (in ms) to delay the actual rendering of elements inside the actions container. When enabled the containing `OverviewItem` can be displayed faster. Can be used e.g. to boost performance when rendering `OverviewItemActions` with `hiddenInteractions` set to `true`.
+    -   `delayDisplayChildren` property: set a time (in ms) to delay the actual rendering of elements inside the actions container. When enabled the containing `OverviewItem` can be displayed faster. Can be used e.g. to boost performance when rendering `OverviewItemActions` with `hiddenInteractions` set to `true`.
     -   `delaySkeleton` property to set the placeholder/skeleton as long as the delayed display is waiting to get processed
 -   `intent` property to `Button`, `FieldItem`, `FieldSet`, `Notification`, and `Spinner`
 

src/components/ContextOverlay/ContextOverlay.tsx

@@ -38,38 +38,44 @@ export const ContextOverlay = ({
     ...otherPopoverProps
 }: ContextOverlayProps) => {
     const placeholderRef = React.useRef(null);
-    const eventmemory = React.useRef<undefined | "afterhover" | "afterfocus">(undefined);
+    const eventMemory = React.useRef<undefined | "afterhover" | "afterfocus">(undefined);
     const [placeholder, setPlaceholder] = React.useState<boolean>(
         // use placeholder only for "simple" overlays without special states
-        otherPopoverProps.disabled !== true &&
-            otherPopoverProps.defaultIsOpen !== true &&
-            otherPopoverProps.isOpen !== true &&
-            typeof otherPopoverProps.renderTarget === "undefined" &&
+        !otherPopoverProps.disabled &&
+            !otherPopoverProps.defaultIsOpen &&
+            !otherPopoverProps.isOpen &&
+            otherPopoverProps.renderTarget === undefined &&
             usePlaceholder
     );
 
-    const targetClassName = `${eccgui}-contextoverlay` + (className ? ` ${className}` : "");
-
     React.useEffect(() => {
         if (placeholderRef.current) {
             const swap = (ev: MouseEvent | globalThis.FocusEvent) => {
-                eventmemory.current = ev.type === "focusin" ? "afterfocus" : "afterhover";
+                eventMemory.current = ev.type === "focusin" ? "afterfocus" : "afterhover";
                 setPlaceholder(false);
             };
             (placeholderRef.current as HTMLElement).addEventListener("mouseenter", swap);
             (placeholderRef.current as HTMLElement).addEventListener("focusin", swap);
+            return () => {
+                if (placeholderRef.current) {
+                    (placeholderRef.current as HTMLElement).removeEventListener("mouseenter", swap);
+                    (placeholderRef.current as HTMLElement).removeEventListener("focusin", swap);
+                }
+            };
         }
     }, [!!placeholderRef.current]);
 
     const refocus = React.useCallback((node) => {
-        if (eventmemory.current === "afterfocus" && node) {
+        if (eventMemory.current === "afterfocus" && node) {
             const target = node.targetRef.current.children[0];
             if (target) {
                 target.focus();
             }
         }
     }, []);
 
+    const targetClassName = `${eccgui}-contextoverlay` + (className ? ` ${className}` : "");
+
     const displayPlaceholder = () => {
         const PlaceholderElement = otherPopoverProps?.targetTagName ?? (otherPopoverProps?.fill ? "div" : "span");
         const childTarget = BlueprintUtils.ensureElement(React.Children.toArray(children)[0]);

src/components/OverviewItem/OverviewItemActions.tsx

@@ -10,6 +10,9 @@ export interface OverviewItemActionsProps extends React.HTMLAttributes<HTMLDivEl
     /**
      * Delay the rendering of the children by a time in milliseconds.
      * Could be used to prevent browser freezes for the initial `OverviewItem` rendering.
+     * In general, it is better to fix the cause, i.e. action elements that are expensive to initialize/render should be
+     * optimized or replaced etc. This workaround only prevents the browser from getting blocked completely and does NOT
+     * solve the actual performance issue.
      */
     delayDisplayChildren?: number;
     /**

src/components/Tooltip/Tooltip.tsx

@@ -57,15 +57,15 @@ export const Tooltip = ({
     ...otherTooltipProps
 }: TooltipProps) => {
     const placeholderRef = React.useRef(null);
-    const eventmemory = React.useRef<null | "afterhover" | "afterfocus">(null);
+    const eventMemory = React.useRef<null | "afterhover" | "afterfocus">(null);
     const searchId = React.useRef<null | string>(null);
-    const swapdelayTime = 100;
+    const swapDelayTime = 100;
     const [placeholder, setPlaceholder] = React.useState<boolean>(
-        otherTooltipProps.disabled !== true &&
-            otherTooltipProps.defaultIsOpen !== true &&
-            otherTooltipProps.isOpen !== true &&
-            typeof otherTooltipProps.renderTarget === "undefined" &&
-            hoverOpenDelay > swapdelayTime &&
+        !otherTooltipProps.disabled &&
+            !otherTooltipProps.defaultIsOpen &&
+            !otherTooltipProps.isOpen &&
+            otherTooltipProps.renderTarget === undefined &&
+            hoverOpenDelay > swapDelayTime &&
             (usePlaceholder === true || (typeof content === "string" && usePlaceholder !== false))
     );
 
@@ -77,33 +77,39 @@ export const Tooltip = ({
     React.useEffect(() => {
         if (placeholderRef.current !== null) {
             const swap = (ev: MouseEvent | globalThis.FocusEvent) => {
-                const swapdelay = setTimeout(() => {
+                const swapDelay = setTimeout(() => {
                     // we delay the swap to prevent unwanted effects
-                    // (e.g. forced mouseover after the swap but the curser is already somewhere else)
-                    eventmemory.current = ev.type === "focusin" ? "afterfocus" : "afterhover";
+                    // (e.g. forced mouseover after the swap but the cursor is already somewhere else)
+                    eventMemory.current = ev.type === "focusin" ? "afterfocus" : "afterhover";
                     searchId.current = Date.now().toString(16) + Math.random().toString(16).slice(2);
                     setPlaceholder(false);
-                }, swapdelayTime);
+                }, swapDelayTime);
                 if (placeholderRef.current !== null)
                     (placeholderRef.current as HTMLElement).addEventListener(
                         ev.type === "focusin" ? "focusout" : "mouseleave",
-                        () => clearTimeout(swapdelay)
+                        () => clearTimeout(swapDelay)
                     );
             };
             (placeholderRef.current as HTMLElement).addEventListener("mouseenter", swap);
             (placeholderRef.current as HTMLElement).addEventListener("focusin", swap);
+            return () => {
+                if (placeholderRef.current) {
+                    (placeholderRef.current as HTMLElement).removeEventListener("mouseenter", swap);
+                    (placeholderRef.current as HTMLElement).removeEventListener("focusin", swap);
+                }
+            };
         }
     }, [!!placeholderRef.current]);
 
     const refocus = React.useCallback((node) => {
-        if (eventmemory.current && node) {
+        if (eventMemory.current && node) {
             // we do not have a `targetRef` here, so we need to workaround it
             // const target = node.targetRef.current.children[0];
             const target = document.body.querySelector(
-                `[data-postplaceholder=id${eventmemory.current}${searchId.current}]`
+                `[data-postplaceholder=id${eventMemory.current}${searchId.current}]`
             )?.children[0];
             if (target) {
-                switch (eventmemory.current) {
+                switch (eventMemory.current) {
                     case "afterfocus":
                         (target as HTMLElement).focus();
                         break;
@@ -152,7 +158,7 @@ export const Tooltip = ({
     ) : (
         <BlueprintTooltip
             lazy={true}
-            hoverOpenDelay={hoverOpenDelay - swapdelayTime}
+            hoverOpenDelay={hoverOpenDelay - swapDelayTime}
             {...otherTooltipProps}
             content={tooltipContent}
             className={targetClassName}
@@ -165,7 +171,7 @@ export const Tooltip = ({
             targetProps={
                 {
                     ...otherTooltipProps.targetProps,
-                    "data-postplaceholder": `id${eventmemory.current}${searchId.current}`,
+                    "data-postplaceholder": `id${eventMemory.current}${searchId.current}`,
                 } as React.HTMLProps<HTMLElement>
             }
         >