Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contextual move #558

Merged
merged 24 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2f33993
Added basen modal for moveinbox items
madhav-relish Nov 6, 2024
3be6c36
Move to space fucntionality added via a modal
madhav-relish Nov 6, 2024
e5d9f77
Added search feature for spaces and added a toast message for user
madhav-relish Nov 6, 2024
ccd2615
Resolved the warning
madhav-relish Nov 6, 2024
9d2419d
Fixed the input focus
madhav-relish Nov 6, 2024
143f92c
Fixed lints
madhav-relish Nov 6, 2024
2cb2b3d
Added user notification if error happens
madhav-relish Nov 6, 2024
b11b6ec
Resolved conflicts
madhav-relish Nov 13, 2024
7949e98
Updated the logic of move item
madhav-relish Nov 13, 2024
ab19615
Fixed the linting errors
madhav-relish Nov 13, 2024
9503ac6
Added move item to another item description
madhav-relish Nov 13, 2024
943295b
Added some styles for tiptap
madhav-relish Nov 13, 2024
13c73e0
Remove item from list when moved, removed logs and comments
madhav-relish Nov 13, 2024
38d476d
Improved modal styling
madhav-relish Nov 13, 2024
53a845a
Added saerch for items also
madhav-relish Nov 13, 2024
ae9c9b0
Fixed the css issue
madhav-relish Nov 13, 2024
13527eb
Added updation logic
madhav-relish Nov 13, 2024
44a1895
Fixed lnting errors
madhav-relish Nov 13, 2024
5d8b358
Added coderabbit suggestions
madhav-relish Nov 13, 2024
9b3e294
Fixed linting and build errors
madhav-relish Nov 13, 2024
2a9be52
Added better UI texts
madhav-relish Nov 13, 2024
cb44eb2
better messages
madhav-relish Nov 13, 2024
173b8a1
Merge branch 'preview' of https://github.com/marchhq/march into conte…
madhav-relish Nov 14, 2024
0fe8d1d
Added more error handling
madhav-relish Nov 14, 2024
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
197 changes: 197 additions & 0 deletions apps/frontend/src/components/Inbox/MoveInboxItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
"use client"

import React, { useEffect, useRef, useState } from "react"

import { DialogHeader, DialogTitle } from "../ui/dialog"
import { Input } from "../ui/input"
import { useAuth } from "@/src/contexts/AuthContext"
import { useModal } from "@/src/contexts/ModalProvider"
import { useToast } from "@/src/hooks/use-toast"
import { CycleItem } from "@/src/lib/@types/Items/Cycle"
import { useCycleItemStore } from "@/src/lib/store/cycle.store"
import useSpaceStore from "@/src/lib/store/space.store"

type Props = {
inboxItemId: string
}

const MoveInboxItem = ({ inboxItemId }: Props) => {
const { session } = useAuth()
const { hideModal } = useModal()
const inputRef = useRef<HTMLInputElement>(null)
const [searchTerm, setSearchTerm] = useState<string>("")
const { spaces, fetchSpaces, loading: isSpaceLoading } = useSpaceStore()
const {
updateItem,
fetchInbox,
items,
isLoading: isInboxItemsLoading,
} = useCycleItemStore()
const [modalItems, setModalItems] = useState<CycleItem[]>(items)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Initialize 'modalItems' to an empty array if 'items' is undefined

If items is undefined initially, modalItems will also be undefined, leading to runtime errors when calling modalItems.filter.

Apply this diff to ensure modalItems is initialized properly:

- const [modalItems, setModalItems] = useState<CycleItem[]>(items)
+ const [modalItems, setModalItems] = useState<CycleItem[]>(items || [])
📝 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
const [modalItems, setModalItems] = useState<CycleItem[]>(items)
const [modalItems, setModalItems] = useState<CycleItem[]>(items || [])

const { toast } = useToast()

useEffect(() => {
if (!spaces || spaces.length === 0) {
void fetchSpaces(session)
}
if (!items || items.length === 0) {
void fetchInbox(session)
}
// Filter out the selected inbox item from the list
const availableInboxItems = items.filter((item) => item._id !== inboxItemId)
setModalItems(availableInboxItems)
}, [fetchSpaces, session, spaces, items])

// Focus the input field when the modal opens
useEffect(() => {
if (inputRef.current) {
inputRef.current.focus()
}
}, [])

const handleSpaceClick = async (spaceId: string) => {
try {
await updateItem(session, { spaces: [spaceId] }, inboxItemId)
await fetchInbox(session)
toast({ title: "🚀 Moved successfully!" })
hideModal()
} catch (error) {
toast({ title: "Oops something seems wrong!", variant: "destructive" })
console.error("Failed to move item:", error)
}
}

const handleItemClick = async (
destiantionItemID: string,
destinationItemDescription: string
Comment on lines +67 to +68
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in variable name 'destiantionItemID'

The variable destiantionItemID is misspelled. It should be destinationItemID to maintain consistency and avoid confusion.

Apply this diff to correct the typo:

-    destiantionItemID: string,
+    destinationItemID: string,
     destinationItemDescription: string
📝 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
destiantionItemID: string,
destinationItemDescription: string
destinationItemID: string,
destinationItemDescription: string

) => {
try {
const sourceItem: CycleItem[] = items.filter(
(item) => item._id === inboxItemId
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure 'sourceItem' is not empty before accessing 'sourceItem[0]'

To prevent potential runtime errors, verify that sourceItem contains at least one item before accessing sourceItem[0].

Apply this diff to add a check for sourceItem length:

 const sourceItem: CycleItem[] = items.filter(
   (item) => item._id === inboxItemId
 )
+ if (sourceItem.length === 0) {
+   throw new Error('Source item not found');
+ }
📝 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
const sourceItem: CycleItem[] = items.filter(
(item) => item._id === inboxItemId
)
const sourceItem: CycleItem[] = items.filter(
(item) => item._id === inboxItemId
)
if (sourceItem.length === 0) {
throw new Error('Source item not found');
}

await updateItem(
session,
{
description: `
<p>${destinationItemDescription}</p>
<ul data-type="taskList">
<li data-checked="false" data-type="taskItem">
<label><input type="checkbox"><span></span></label>
<div data-indent="true">
<p>${sourceItem[0].title}</p>
</div>
</li>
</ul>
<span data-indent="true">${sourceItem[0].description}</span>
`,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Sanitize user input in HTML strings to prevent XSS vulnerabilities

When constructing HTML content with user-provided data, ensure that inputs like destinationItemDescription, sourceItem[0].title, and sourceItem[0].description are properly sanitized to prevent XSS attacks.

Consider using a library or utility function to sanitize these inputs before including them in the HTML string.

},
destiantionItemID
)

//Remove the item from the main list
await updateItem(session, { isDeleted: true }, inboxItemId)
await fetchInbox(session)
toast({ title: "🚀 Moved successfully!" })
hideModal()
} catch (error) {
console.error(
"Error file moving item to another item description::",
error
)
toast({ title: "Oops something seems wrong!", variant: "destructive" })
}
}

// Filter spaces based on the search term
const filteredSpaces = spaces.filter((space) =>
space.name.toLowerCase().includes(searchTerm.toLowerCase())
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle undefined 'spaces' to prevent runtime errors

If spaces is undefined, calling spaces.filter will cause a runtime error. Initialize spaces to an empty array or guard against undefined.

Apply this diff to guard against undefined spaces:

- const filteredSpaces = spaces.filter((space) =>
+ const filteredSpaces = (spaces || []).filter((space) =>
     space.name.toLowerCase().includes(searchTerm.toLowerCase())
 )
📝 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
const filteredSpaces = spaces.filter((space) =>
space.name.toLowerCase().includes(searchTerm.toLowerCase())
)
const filteredSpaces = (spaces || []).filter((space) =>
space.name.toLowerCase().includes(searchTerm.toLowerCase())
)


//Filter items based on search term
const filteredItems = modalItems.filter((item) =>
item.title.toLowerCase().includes(searchTerm.toLowerCase())
)

return (
<>
<DialogHeader className="sr-only h-0 p-0">
<DialogTitle className="sr-only p-0"></DialogTitle>
</DialogHeader>
<div className="flex justify-between gap-2 px-4 pt-1 text-xs text-secondary-foreground">
<span className="flex-1 truncate text-primary-foreground">
<Input
ref={inputRef}
className="w-full border-none px-0 text-primary-foreground outline-none placeholder:text-secondary-foreground"
placeholder="Specify the target item: todo, note, event etc"
aria-label="Search spaces or items"
value={searchTerm}
onChange={(e) => setSearchTerm(e.target.value)}
disabled={isSpaceLoading || isInboxItemsLoading}
/>
</span>
</div>
<div className="flex items-center gap-5 bg-transparent text-secondary-foreground">
<div className="flex h-fit min-w-[350px] flex-col gap-5 overflow-hidden rounded-lg bg-background p-5 py-0 text-sm">
<div className="flex max-h-48 flex-col gap-1.5 overflow-y-auto">
{isInboxItemsLoading && <div>Loading Inbox Items...</div>}
{filteredItems.length > 0 ? (
filteredItems?.map((item) => (
<button
key={item._id}
className="cursor-pointer text-left hover:text-primary-foreground"
onClick={() => handleItemClick(item._id, item.description)}
onKeyDown={(e) => {
if (e.key === "Enter") {
handleItemClick(item._id, item.description)
}
}}
type="button"
disabled={isInboxItemsLoading}
aria-label={`Move to ${item.title}`}
>
{item.title}
</button>
))
) : (
<div>No matching results found!</div>
)}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Prevent 'No matching results found!' message during loading

Ensure that the 'No matching results found!' message is displayed only after loading is complete and no results are found, to avoid confusing the user.

Apply this diff to adjust the conditional rendering:

 {isInboxItemsLoading && <div>Loading Inbox Items...</div>}
-{filteredItems.length > 0 ? (
+{!isInboxItemsLoading && filteredItems.length > 0 ? (
   filteredItems.map((item) => (
     // ...
   ))
 ) : (
-  <div>No matching results found!</div>
+  !isInboxItemsLoading && <div>No matching results found!</div>
 )}
📝 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
<div className="flex max-h-48 flex-col gap-1.5 overflow-y-auto">
{isInboxItemsLoading && <div>Loading Inbox Items...</div>}
{filteredItems.length > 0 ? (
filteredItems?.map((item) => (
<button
key={item._id}
className="cursor-pointer text-left hover:text-primary-foreground"
onClick={() => handleItemClick(item._id, item.description)}
onKeyDown={(e) => {
if (e.key === "Enter") {
handleItemClick(item._id, item.description)
}
}}
type="button"
disabled={isInboxItemsLoading}
aria-label={`Move to ${item.title}`}
>
{item.title}
</button>
))
) : (
<div>No matching results found!</div>
)}
<div className="flex max-h-48 flex-col gap-1.5 overflow-y-auto">
{isInboxItemsLoading && <div>Loading Inbox Items...</div>}
{!isInboxItemsLoading && filteredItems.length > 0 ? (
filteredItems?.map((item) => (
<button
key={item._id}
className="cursor-pointer text-left hover:text-primary-foreground"
onClick={() => handleItemClick(item._id, item.description)}
onKeyDown={(e) => {
if (e.key === "Enter") {
handleItemClick(item._id, item.description)
}
}}
type="button"
disabled={isInboxItemsLoading}
aria-label={`Move to ${item.title}`}
>
{item.title}
</button>
))
) : (
!isInboxItemsLoading && <div>No matching results found!</div>
)}

</div>
</div>
</div>
<div className="flex w-full items-center gap-2 px-2 text-secondary-foreground">
<span>Spaces</span>
<span className="h-px w-full bg-secondary-foreground"></span>
</div>
<div className="flex items-center gap-5 bg-transparent text-secondary-foreground">
<div className="flex h-fit min-w-[350px] flex-col gap-5 overflow-hidden rounded-lg bg-background p-5 pt-0 text-sm">
<div className="flex max-h-48 flex-col gap-1.5 overflow-y-auto">
{isSpaceLoading && <div>Loading spaces...</div>}
{filteredSpaces.length > 0 ? (
filteredSpaces.map((space) => (
<button
key={space._id}
className="cursor-pointer text-left hover:text-primary-foreground"
onClick={() => handleSpaceClick(space._id)}
onKeyDown={(e) => {
if (e.key === "Enter") {
handleSpaceClick(space._id)
}
}}
disabled={isSpaceLoading}
aria-label={`Move to ${space.name}`}
type="button"
>
{space.name}
</button>
))
) : (
<div>No matching results found!</div>
)}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Prevent 'No matching results found!' message during loading

Similarly, ensure that the 'No matching results found!' message for spaces is displayed only after loading is complete.

Apply this diff to adjust the conditional rendering:

 {isSpaceLoading && <div>Loading spaces...</div>}
-{filteredSpaces.length > 0 ? (
+{!isSpaceLoading && filteredSpaces.length > 0 ? (
   filteredSpaces.map((space) => (
     // ...
   ))
 ) : (
-  <div>No matching results found!</div>
+  !isSpaceLoading && <div>No matching results found!</div>
 )}
📝 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
{isSpaceLoading && <div>Loading spaces...</div>}
{filteredSpaces.length > 0 ? (
filteredSpaces.map((space) => (
<button
key={space._id}
className="cursor-pointer text-left hover:text-primary-foreground"
onClick={() => handleSpaceClick(space._id)}
onKeyDown={(e) => {
if (e.key === "Enter") {
handleSpaceClick(space._id)
}
}}
disabled={isSpaceLoading}
aria-label={`Move to ${space.name}`}
type="button"
>
{space.name}
</button>
))
) : (
<div>No matching results found!</div>
)}
{isSpaceLoading && <div>Loading spaces...</div>}
{!isSpaceLoading && filteredSpaces.length > 0 ? (
filteredSpaces.map((space) => (
<button
key={space._id}
className="cursor-pointer text-left hover:text-primary-foreground"
onClick={() => handleSpaceClick(space._id)}
onKeyDown={(e) => {
if (e.key === "Enter") {
handleSpaceClick(space._id)
}
}}
disabled={isSpaceLoading}
aria-label={`Move to ${space.name}`}
type="button"
>
{space.name}
</button>
))
) : (
!isSpaceLoading && <div>No matching results found!</div>
)}

</div>
</div>
</div>
</>
)
}

export default MoveInboxItem
17 changes: 16 additions & 1 deletion apps/frontend/src/components/atoms/ItemList.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import React from "react"

import { CalendarIcon, GithubIcon, MailsIcon } from "lucide-react"
import { CalendarIcon, GithubIcon, MailsIcon, MoveIcon } from "lucide-react"
import Image from "next/image"

import MoveInboxItem from "../Inbox/MoveInboxItem"
import BoxIcon from "@/public/icons/box.svg"
import BoxFilledIcon from "@/public/icons/boxfilled.svg"
import LinearIcon from "@/public/icons/linear.svg"
import { useModal } from "@/src/contexts/ModalProvider"
import { CycleItem } from "@/src/lib/@types/Items/Cycle"
import classNames from "@/src/utils/classNames"

Expand Down Expand Up @@ -55,6 +57,7 @@ export const ItemList: React.FC<ItemListProps> = ({
isOverdue = false,
doneLine = false,
}) => {
const { showModal } = useModal()
return (
<>
{items.map((item) => (
Expand Down Expand Up @@ -118,6 +121,18 @@ export const ItemList: React.FC<ItemListProps> = ({
handleRescheduleCalendar(e, item._id, item.dueDate)
}
/>
<MoveIcon
size={14}
className="hover-text"
aria-label="Move item to different space"
role="button"
data-tooltip-id="item-actions"
data-tooltip-content="Move to space"
onClick={(e) => {
e.stopPropagation()
showModal(<MoveInboxItem inboxItemId={item._id} />)
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

MoveInboxItem component does not implement available spaces.

The component is missing the functionality to display available spaces as required by the PR objectives.

  • availableSpaces or related implementations are absent in apps/frontend/src/components/Inbox/MoveInboxItem.tsx.
🔗 Analysis chain

Verify MoveInboxItem component implementation.

Let's verify that the MoveInboxItem component exists and implements all required features from the PR objectives.


Locate MoveInboxItem component implementation across the repository.


Verify MoveInboxItem component implements available spaces and search functionality.

[/run_scripts]

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify MoveInboxItem component implementation

# Check if the component file exists
echo "Checking for MoveInboxItem component..."
fd "MoveInboxItem.tsx" src/components/Inbox

# Verify component implements required features
echo "Verifying component features..."
rg -A 10 "const MoveInboxItem" src/components/Inbox

# Check for search functionality
echo "Checking search implementation..."
rg -l "search|filter" src/components/Inbox/MoveInboxItem

Length of output: 639


Script:

#!/bin/bash
# Description: Locate MoveInboxItem component implementation

# Search for MoveInboxItem.tsx in the entire repository
fd "MoveInboxItem.tsx"

Length of output: 75


Script:

#!/bin/bash
# Description: Verify MoveInboxItem component implements required features

# Search for available spaces implementation
echo "Checking for available spaces implementation..."
rg "availableSpaces|spacesList|renderSpaces" apps/frontend/src/components/Inbox/MoveInboxItem.tsx

# Search for search functionality implementation
echo "Checking for search functionality implementation..."
rg "search|filter|useSearch" apps/frontend/src/components/Inbox/MoveInboxItem.tsx

Length of output: 730

}}
/>
</div>
</button>
))}
Expand Down
Loading