fix render-if-visible-hoc's style calculation performance issue (#5647)

makeplane · Sep 19, 2024 · c8c9638 · c8c9638
1 parent bd0ca0c
commit c8c9638
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 20 deletions.
diff --git a/web/core/components/core/render-if-visible-HOC.tsx b/web/core/components/core/render-if-visible-HOC.tsx
@@ -11,6 +11,7 @@ type Props = {
   classNames?: string;
   placeholderChildren?: ReactNode;
   defaultValue?: boolean;
+  shouldRecordHeights?: boolean;
   useIdletime?: boolean;
 };
 
@@ -23,6 +24,8 @@ const RenderIfVisible: React.FC<Props> = (props) => {
     as = "div",
     children,
     classNames = "",
+    shouldRecordHeights = true,
+    //placeholder children
     placeholderChildren = null, //placeholder children
     defaultValue = false,
     useIdletime = false,
@@ -64,15 +67,15 @@ const RenderIfVisible: React.FC<Props> = (props) => {
 
   //Set height after render
   useEffect(() => {
-    if (intersectionRef.current && isVisible) {
-      placeholderHeight.current = `${intersectionRef.current.offsetHeight}px`;
+    if (intersectionRef.current && isVisible && shouldRecordHeights) {
+      window.requestIdleCallback(() => {
+        if (intersectionRef.current) placeholderHeight.current = `${intersectionRef.current.offsetHeight}px`;
+      });
     }
-  }, [isVisible, intersectionRef]);
+  }, [isVisible, intersectionRef, shouldRecordHeights]);
 
   const child = isVisible ? <>{children}</> : placeholderChildren;
-  const style: { width?: string; height?: string } = isVisible
-    ? {}
-    : { height: placeholderHeight.current, width: "100%" };
+  const style = isVisible || placeholderChildren ? {} : { height: placeholderHeight.current, width: "100%" };
   const className = isVisible || placeholderChildren ? classNames : cn(classNames, "bg-custom-background-80");
 
   return React.createElement(as, { ref: intersectionRef, style, className }, child);

diff --git a/web/core/components/issues/issue-layouts/list/block-root.tsx b/web/core/components/issues/issue-layouts/list/block-root.tsx
@@ -13,9 +13,11 @@ import { IIssueDisplayProperties, TIssue, TIssueMap } from "@plane/types";
 import { DropIndicator } from "@plane/ui";
 import RenderIfVisible from "@/components/core/render-if-visible-HOC";
 import { IssueBlock } from "@/components/issues/issue-layouts/list";
+import { ListLoaderItemRow } from "@/components/ui";
 // hooks
 import { useIssueDetail } from "@/hooks/store";
 import { TSelectionHelper } from "@/hooks/use-multiple-select";
+import { usePlatformOS } from "@/hooks/use-platform-os";
 // types
 import { HIGHLIGHT_CLASS, getIssueBlockId, isIssueNew } from "../utils";
 import { TRenderQuickActions } from "./list-view-types";
@@ -66,6 +68,8 @@ export const IssueBlockRoot: FC<Props> = observer((props) => {
   const [isCurrentBlockDragging, setIsCurrentBlockDragging] = useState(false);
   // ref
   const issueBlockRef = useRef<HTMLDivElement | null>(null);
+  // hooks
+  const { isMobile } = usePlatformOS();
   // store hooks
   const { subIssues: subIssuesStore } = useIssueDetail();
 
@@ -125,11 +129,14 @@ export const IssueBlockRoot: FC<Props> = observer((props) => {
       <DropIndicator classNames={"absolute top-0 z-[2]"} isVisible={instruction === "DRAG_OVER"} />
       <RenderIfVisible
         key={`${issueId}`}
-        defaultHeight="3rem"
         root={containerRef}
         classNames={`relative ${isLastChild && !isExpanded ? "" : "border-b border-b-custom-border-200"}`}
         verticalOffset={100}
         defaultValue={shouldRenderByDefault || isIssueNew(issuesMap[issueId])}
+        placeholderChildren={
+          <ListLoaderItemRow shouldAnimate={false} renderForPlaceHolder={true} defaultPropertyCount={4} />
+        }
+        shouldRecordHeights={isMobile}
       >
         <IssueBlock
           issueId={issueId}

diff --git a/web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx b/web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx
@@ -76,16 +76,20 @@ export const SpreadsheetIssueRow = observer((props: Props) => {
       {/* first column/ issue name and key column */}
       <RenderIfVisible
         as="tr"
-        defaultHeight="calc(2.75rem - 1px)"
         root={containerRef}
         placeholderChildren={
-          <td colSpan={100} className="border-[0.5px] border-transparent border-b-custom-border-200" />
+          <td
+            colSpan={100}
+            className="border-[0.5px] border-transparent border-b-custom-border-200"
+            style={{ height: "calc(2.75rem - 1px)" }}
+          />
         }
         classNames={cn("bg-custom-background-100 transition-[background-color]", {
           "group selected-issue-row": isIssueSelected,
           "border-[0.5px] border-custom-border-400": isIssueActive,
         })}
         verticalOffset={100}
+        shouldRecordHeights={false}
       >
         <IssueRowDetails
           issueId={issueId}

diff --git a/web/core/components/ui/loader/layouts/list-layout-loader.tsx b/web/core/components/ui/loader/layouts/list-layout-loader.tsx
@@ -1,20 +1,51 @@
 import { Fragment, forwardRef } from "react";
+import { cn } from "@plane/editor";
 import { Row } from "@plane/ui";
 import { getRandomInt, getRandomLength } from "../utils";
 
-export const ListLoaderItemRow = forwardRef<HTMLDivElement>((props, ref) => (
-  <Row ref={ref} className="flex items-center justify-between h-11 py-3 border-b border-custom-border-200">
+export const ListLoaderItemRow = forwardRef<
+  HTMLDivElement,
+  { shouldAnimate?: boolean; renderForPlaceHolder?: boolean; defaultPropertyCount?: number }
+>(({ shouldAnimate = true, renderForPlaceHolder = false, defaultPropertyCount = 6 }, ref) => (
+  <Row
+    ref={ref}
+    className={cn("flex items-center justify-between h-11 py-3 ", {
+      "bg-custom-background-100": renderForPlaceHolder,
+      "border-b border-custom-border-200": !renderForPlaceHolder,
+    })}
+  >
     <div className="flex items-center gap-3">
-      <span className="h-5 w-10 bg-custom-background-80 rounded animate-pulse" />
-      <span className={`h-5 w-${getRandomLength(["32", "52", "72"])} bg-custom-background-80 rounded animate-pulse`} />
+      <span
+        className={cn("h-5 w-10 bg-custom-background-80 rounded", {
+          "animate-pulse": shouldAnimate,
+          "bg-custom-background-90": renderForPlaceHolder,
+        })}
+      />
+      <span
+        className={cn(`h-5 w-${getRandomLength(["32", "52", "72"])} bg-custom-background-80 rounded`, {
+          "animate-pulse": shouldAnimate,
+          "bg-custom-background-90": renderForPlaceHolder,
+        })}
+      />
     </div>
     <div className="flex items-center gap-2">
-      {[...Array(6)].map((_, index) => (
+      {[...Array(defaultPropertyCount)].map((_, index) => (
         <Fragment key={index}>
           {getRandomInt(1, 2) % 2 === 0 ? (
-            <span key={index} className="h-5 w-5 bg-custom-background-80 rounded animate-pulse" />
+            <span
+              key={index}
+              className={cn("h-5 w-5 bg-custom-background-80 rounded", {
+                "animate-pulse": shouldAnimate,
+                "bg-custom-background-90": renderForPlaceHolder,
+              })}
+            />
           ) : (
-            <span className="h-5 w-16 bg-custom-background-80 rounded animate-pulse" />
+            <span
+              className={cn("h-5 w-16 bg-custom-background-80 rounded", {
+                "animate-pulse": shouldAnimate,
+                "bg-custom-background-90": renderForPlaceHolder,
+              })}
+            />
           )}
         </Fragment>
       ))}

diff --git a/web/core/store/issue/issue.store.ts b/web/core/store/issue/issue.store.ts
@@ -77,7 +77,7 @@ export class IssueStore implements IIssueStore {
    * @returns {void}
    */
   updateIssue = (issueId: string, issue: Partial<TIssue>) => {
-    if (!issue || !issueId || isEmpty(this.issuesMap) || !this.issuesMap[issueId]) return;
+    if (!issue || !issueId || !this.issuesMap[issueId]) return;
     runInAction(() => {
       set(this.issuesMap, [issueId, "updated_at"], getCurrentDateTimeInISO());
       Object.keys(issue).forEach((key) => {
@@ -92,7 +92,7 @@ export class IssueStore implements IIssueStore {
    * @returns {void}
    */
   removeIssue = (issueId: string) => {
-    if (!issueId || isEmpty(this.issuesMap) || !this.issuesMap[issueId]) return;
+    if (!issueId || !this.issuesMap[issueId]) return;
     runInAction(() => {
       delete this.issuesMap[issueId];
     });
@@ -105,7 +105,7 @@ export class IssueStore implements IIssueStore {
    * @returns {TIssue | undefined}
    */
   getIssueById = computedFn((issueId: string) => {
-    if (!issueId || isEmpty(this.issuesMap) || !this.issuesMap[issueId]) return undefined;
+    if (!issueId || !this.issuesMap[issueId]) return undefined;
     return this.issuesMap[issueId];
   });
 
@@ -116,7 +116,7 @@ export class IssueStore implements IIssueStore {
    * @returns {Record<string, TIssue> | undefined}
    */
   getIssuesByIds = computedFn((issueIds: string[], type: "archived" | "un-archived") => {
-    if (!issueIds || issueIds.length <= 0 || isEmpty(this.issuesMap)) return [];
+    if (!issueIds || issueIds.length <= 0) return [];
     const filteredIssues: TIssue[] = [];
     Object.values(issueIds).forEach((issueId) => {
       // if type is archived then check archived_at is not null