Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions web/core/components/stickies/delete-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { AlertModalCore, TOAST_TYPE, setToast } from "@plane/ui";

interface IStickyDelete {
isOpen: boolean;
handleSubmit: () => void;
handleSubmit: () => Promise<void>;
handleClose: () => void;
}

Expand All @@ -20,11 +20,11 @@ export const StickyDeleteModal: React.FC<IStickyDelete> = observer((props) => {
try {
setLoader(true);
await handleSubmit();
} catch (error) {
} catch {
setToast({
type: TOAST_TYPE.ERROR,
title: "Warning!",
message: "Something went wrong please try again later.",
message: "Something went wrong. Please try again later.",
});
} finally {
setLoader(false);
Expand All @@ -38,7 +38,7 @@ export const StickyDeleteModal: React.FC<IStickyDelete> = observer((props) => {
isSubmitting={loader}
isOpen={isOpen}
title="Delete sticky"
content={<>Are you sure you want to delete the sticky? </>}
content="Are you sure you want to delete the sticky?"
/>
);
});
26 changes: 14 additions & 12 deletions web/core/components/stickies/layout/stickies-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,17 @@ import type { ElementDragPayload } from "@atlaskit/pragmatic-drag-and-drop/eleme
import { observer } from "mobx-react";
import { usePathname } from "next/navigation";
import Masonry from "react-masonry-component";
// plane ui
import { Loader } from "@plane/ui";
// components
import { EmptyState } from "@/components/empty-state";
import { StickiesEmptyState } from "@/components/home/widgets/empty-states/stickies";
// constants
import { EmptyStateType } from "@/constants/empty-state";
// hooks
import { useSticky } from "@/hooks/use-stickies";
import { useStickyOperations } from "../sticky/use-operations";
import { StickiesLoader } from "./stickies-loader";
import { StickyDNDWrapper } from "./sticky-dnd-wrapper";
import { getInstructionFromPayload } from "./sticky.helpers";
import { StickiesEmptyState } from "@/components/home/widgets/empty-states/stickies";

type TStickiesLayout = {
workspaceSlug: string;
Expand All @@ -42,6 +41,14 @@ export const StickiesList = observer((props: TProps) => {
const itemWidth = `${100 / columnCount}%`;
const totalRows = Math.ceil(workspaceStickyIds.length / columnCount);
const isStickiesPage = pathname?.includes("stickies");
const masonryRef = useRef<any>(null);

const handleLayout = () => {
if (masonryRef.current) {
// Force reflow
masonryRef.current.performLayout();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaryan610 could you please let me know what is happening here? cant find mdn docs for it? going by your comment are you trying to force layout reflow? if that is the case it is usually not a good practice since it is better to always allow browser decide on forcing layout reflow

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gakshita can you please take this up?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the content addition in sticky was resulting in increment of height then the masonry library wasn't adjusting the layout, resulting in overlap of stickies. Therefore i am doing a force update.

}
};

// Function to determine if an item is in first or last row
const getRowPositions = (index: number) => {
Expand Down Expand Up @@ -72,19 +79,13 @@ export const StickiesList = observer((props: TProps) => {
};

if (loader === "init-loader") {
return (
<div className="min-h-[500px] overflow-scroll pb-2">
<Loader>
<Loader.Item height="300px" width="255px" />
</Loader>
</div>
);
return <StickiesLoader />;
}

if (loader === "loaded" && workspaceStickyIds.length === 0) {
return (
<div className="size-full grid place-items-center">
{isStickiesPage ? (
{isStickiesPage || searchQuery ? (
<EmptyState
type={searchQuery ? EmptyStateType.STICKIES_SEARCH : EmptyStateType.STICKIES}
layout={searchQuery ? "screen-simple" : "screen-detailed"}
Expand All @@ -106,7 +107,7 @@ export const StickiesList = observer((props: TProps) => {
return (
<div className="transition-opacity duration-300 ease-in-out">
{/* @ts-expect-error type mismatch here */}
<Masonry elementType="div">
<Masonry elementType="div" ref={masonryRef}>
Comment on lines 109 to +110
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix type mismatch in Masonry component.

The @ts-expect-error comment indicates a known type issue that should be properly addressed rather than suppressed.

Please investigate the type mismatch and consider:

  1. Updating the type definitions for the Masonry component
  2. Using a properly typed alternative
  3. Creating a proper type declaration file if one doesn't exist

{workspaceStickyIds.map((stickyId, index) => {
const { isInFirstRow, isInLastRow } = getRowPositions(index);
return (
Expand All @@ -119,6 +120,7 @@ export const StickiesList = observer((props: TProps) => {
isLastChild={index === workspaceStickyIds.length - 1}
isInFirstRow={isInFirstRow}
isInLastRow={isInLastRow}
handleLayout={handleLayout}
/>
);
})}
Expand Down
45 changes: 45 additions & 0 deletions web/core/components/stickies/layout/stickies-loader.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// plane ui
import { Loader } from "@plane/ui";

export const StickiesLoader = () => (
<div className="overflow-scroll pb-2 grid grid-cols-4 gap-4">
{Array.from({ length: 4 }).map((_, index) => (
<Loader key={index} className="space-y-5 border border-custom-border-200 p-3 rounded">
<div className="space-y-2">
<Loader.Item height="20px" />
<Loader.Item height="15px" width="75%" />
</div>
<div className="space-y-2">
<div className="flex items-center gap-2">
<Loader.Item height="15px" width="15px" className="flex-shrink-0" />
<Loader.Item height="15px" width="100%" />
</div>
<div className="flex items-center gap-2">
<Loader.Item height="15px" width="15px" className="flex-shrink-0" />
<Loader.Item height="15px" width="75%" />
</div>
<div className="flex items-center gap-2">
<Loader.Item height="15px" width="15px" className="flex-shrink-0" />
<Loader.Item height="15px" width="90%" />
</div>
<div className="flex items-center gap-2">
<Loader.Item height="15px" width="15px" className="flex-shrink-0" />
<Loader.Item height="15px" width="60%" />
</div>
<div className="flex items-center gap-2">
<Loader.Item height="15px" width="15px" className="flex-shrink-0" />
<Loader.Item height="15px" width="50%" />
</div>
</div>
<div className="flex items-center justify-between gap-2">
<div className="flex items-center gap-2">
<Loader.Item height="25px" width="25px" />
<Loader.Item height="25px" width="25px" />
<Loader.Item height="25px" width="25px" />
</div>
<Loader.Item height="25px" width="25px" className="flex-shrink-0" />
</div>
</Loader>
))}
</div>
);
216 changes: 107 additions & 109 deletions web/core/components/stickies/layout/sticky-dnd-wrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,123 +15,121 @@ import { attachInstruction } from "@atlaskit/pragmatic-drag-and-drop-hitbox/tree
import { observer } from "mobx-react";
import { usePathname } from "next/navigation";
import { createRoot } from "react-dom/client";
// plane types
import { InstructionType } from "@plane/types";
// plane ui
import { DropIndicator } from "@plane/ui";
import { cn } from "@plane/utils";
// components
import { StickyNote } from "../sticky";
// helpers
import { getInstructionFromPayload } from "./sticky.helpers";

// Draggable Sticky Wrapper Component
export const StickyDNDWrapper = observer(
({
stickyId,
workspaceSlug,
itemWidth,
isLastChild,
isInFirstRow,
isInLastRow,
handleDrop,
}: {
stickyId: string;
workspaceSlug: string;
itemWidth: string;
isLastChild: boolean;
isInFirstRow: boolean;
isInLastRow: boolean;
handleDrop: (self: DropTargetRecord, source: ElementDragPayload, location: DragLocationHistory) => void;
}) => {
const pathName = usePathname();
const [isDragging, setIsDragging] = useState(false);
const [instruction, setInstruction] = useState<InstructionType | undefined>(undefined);
const elementRef = useRef<HTMLDivElement>(null);
type Props = {
stickyId: string;
workspaceSlug: string;
itemWidth: string;
isLastChild: boolean;
isInFirstRow: boolean;
isInLastRow: boolean;
handleDrop: (self: DropTargetRecord, source: ElementDragPayload, location: DragLocationHistory) => void;
handleLayout: () => void;
};

useEffect(() => {
const element = elementRef.current;
if (!element) return;
export const StickyDNDWrapper = observer((props: Props) => {
const { stickyId, workspaceSlug, itemWidth, isLastChild, isInFirstRow, isInLastRow, handleDrop, handleLayout } =
props;
// states
const [isDragging, setIsDragging] = useState(false);
const [instruction, setInstruction] = useState<InstructionType | undefined>(undefined);
// refs
const elementRef = useRef<HTMLDivElement>(null);
// navigation
const pathname = usePathname();

const initialData = { id: stickyId, type: "sticky" };
useEffect(() => {
const element = elementRef.current;
if (!element) return;

if (pathName.includes("stickies"))
return combine(
draggable({
element,
dragHandle: element,
getInitialData: () => initialData,
onDragStart: () => {
setIsDragging(true);
},
onDrop: () => {
setIsDragging(false);
},
onGenerateDragPreview: ({ nativeSetDragImage }) => {
setCustomNativeDragPreview({
getOffset: pointerOutsideOfPreview({ x: "-200px", y: "0px" }),
render: ({ container }) => {
const root = createRoot(container);
root.render(
<div className="scale-50">
<div className="-m-2 max-h-[150px]">
<StickyNote
className={"w-[290px]"}
workspaceSlug={workspaceSlug.toString()}
stickyId={stickyId}
showToolbar={false}
/>
</div>
const initialData = { id: stickyId, type: "sticky" };

if (pathname.includes("stickies"))
return combine(
draggable({
element,
dragHandle: element,
getInitialData: () => initialData,
onDragStart: () => {
setIsDragging(true);
},
onDrop: () => {
setIsDragging(false);
},
onGenerateDragPreview: ({ nativeSetDragImage }) => {
setCustomNativeDragPreview({
getOffset: pointerOutsideOfPreview({ x: "-200px", y: "0px" }),
render: ({ container }) => {
const root = createRoot(container);
root.render(
<div className="scale-50">
<div className="-m-2 max-h-[150px]">
<StickyNote
className={"w-[290px]"}
workspaceSlug={workspaceSlug.toString()}
stickyId={stickyId}
showToolbar={false}
/>
</div>
);
return () => root.unmount();
},
nativeSetDragImage,
});
},
}),
dropTargetForElements({
element,
canDrop: ({ source }) => source.data?.type === "sticky",
getData: ({ input, element }) => {
const blockedStates: InstructionType[] = ["make-child"];
if (!isLastChild) {
blockedStates.push("reorder-below");
}
</div>
);
return () => root.unmount();
},
Comment on lines +70 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix potential memory leak in drag preview rendering.

The drag preview rendering using createRoot might lead to memory leaks. While the root is unmounted in the cleanup function, any resources created within the preview (like event listeners) might not be properly cleaned up.

Consider refactoring to:

  1. Move the preview component to a separate file
  2. Ensure all resources are cleaned up properly
  3. Use useCallback for the render function to prevent unnecessary re-renders
- render: ({ container }) => {
-   const root = createRoot(container);
-   root.render(
-     <div className="scale-50">
-       <div className="-m-2 max-h-[150px]">
-         <StickyNote
-           className={"w-[290px]"}
-           workspaceSlug={workspaceSlug.toString()}
-           stickyId={stickyId}
-           showToolbar={false}
-         />
-       </div>
-     </div>
-   );
-   return () => root.unmount();
- },
+ render: useCallback(({ container }) => {
+   const root = createRoot(container);
+   const Preview = () => (
+     <div className="scale-50">
+       <div className="-m-2 max-h-[150px]">
+         <StickyNote
+           className={"w-[290px]"}
+           workspaceSlug={workspaceSlug.toString()}
+           stickyId={stickyId}
+           showToolbar={false}
+         />
+       </div>
+     </div>
+   );
+   root.render(<Preview />);
+   return () => {
+     root.unmount();
+     // Add any additional cleanup here
+   };
+ }, [workspaceSlug, stickyId]),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
render: ({ container }) => {
const root = createRoot(container);
root.render(
<div className="scale-50">
<div className="-m-2 max-h-[150px]">
<StickyNote
className={"w-[290px]"}
workspaceSlug={workspaceSlug.toString()}
stickyId={stickyId}
showToolbar={false}
/>
</div>
);
return () => root.unmount();
},
nativeSetDragImage,
});
},
}),
dropTargetForElements({
element,
canDrop: ({ source }) => source.data?.type === "sticky",
getData: ({ input, element }) => {
const blockedStates: InstructionType[] = ["make-child"];
if (!isLastChild) {
blockedStates.push("reorder-below");
}
</div>
);
return () => root.unmount();
},
render: useCallback(({ container }) => {
const root = createRoot(container);
const Preview = () => (
<div className="scale-50">
<div className="-m-2 max-h-[150px]">
<StickyNote
className={"w-[290px]"}
workspaceSlug={workspaceSlug.toString()}
stickyId={stickyId}
showToolbar={false}
/>
</div>
</div>
);
root.render(<Preview />);
return () => {
root.unmount();
// Add any additional cleanup here
};
}, [workspaceSlug, stickyId]),

nativeSetDragImage,
});
},
}),
dropTargetForElements({
element,
canDrop: ({ source }) => source.data?.type === "sticky",
getData: ({ input, element }) => {
const blockedStates: InstructionType[] = ["make-child"];
if (!isLastChild) {
blockedStates.push("reorder-below");
}

return attachInstruction(initialData, {
input,
element,
currentLevel: 1,
indentPerLevel: 0,
mode: isLastChild ? "last-in-group" : "standard",
block: blockedStates,
});
},
onDrag: ({ self, source, location }) => {
const instruction = getInstructionFromPayload(self, source, location);
setInstruction(instruction);
},
onDragLeave: () => {
setInstruction(undefined);
},
onDrop: ({ self, source, location }) => {
setInstruction(undefined);
handleDrop(self, source, location);
},
})
);
}, [stickyId, isDragging]);
return attachInstruction(initialData, {
input,
element,
currentLevel: 1,
indentPerLevel: 0,
mode: isLastChild ? "last-in-group" : "standard",
block: blockedStates,
});
},
onDrag: ({ self, source, location }) => {
const instruction = getInstructionFromPayload(self, source, location);
setInstruction(instruction);
},
onDragLeave: () => {
setInstruction(undefined);
},
onDrop: ({ self, source, location }) => {
setInstruction(undefined);
handleDrop(self, source, location);
},
})
);
}, [handleDrop, isDragging, isLastChild, pathname, stickyId, workspaceSlug]);

return (
<div className="relative" style={{ width: itemWidth }}>
{!isInFirstRow && <DropIndicator isVisible={instruction === "reorder-above"} />}
<div
ref={elementRef}
className={cn("flex min-h-[300px] box-border p-2", {
"opacity-50": isDragging,
})}
>
<StickyNote key={stickyId || "new"} workspaceSlug={workspaceSlug} stickyId={stickyId} />
</div>
{!isInLastRow && <DropIndicator isVisible={instruction === "reorder-below"} />}
</div>
);
}
);
return (
<div className="flex min-h-[300px] box-border p-2 flex-col" style={{ width: itemWidth }}>
{!isInFirstRow && <DropIndicator isVisible={instruction === "reorder-above"} />}
<StickyNote
key={stickyId || "new"}
workspaceSlug={workspaceSlug}
stickyId={stickyId}
handleLayout={handleLayout}
/>
{!isInLastRow && <DropIndicator isVisible={instruction === "reorder-below"} />}
</div>
);
});
Loading
Loading