-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Adjust new task button to create new task #4063
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
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import Image from 'next/image' | |
| import Link from 'next/link' | ||
| import { useParams, usePathname, useRouter } from 'next/navigation' | ||
| import { usePostHog } from 'posthog-js/react' | ||
| import { flushSync } from 'react-dom' | ||
| import { | ||
| Blimp, | ||
| Button, | ||
|
|
@@ -101,12 +102,17 @@ import { usePermissionConfig } from '@/hooks/use-permission-config' | |
| import { useSettingsNavigation } from '@/hooks/use-settings-navigation' | ||
| import { useTaskEvents } from '@/hooks/use-task-events' | ||
| import { SIDEBAR_WIDTH } from '@/stores/constants' | ||
| import { useDraftTaskStore } from '@/stores/draft-tasks/store' | ||
| import { useFolderStore } from '@/stores/folders/store' | ||
| import { useSearchModalStore } from '@/stores/modals/search/store' | ||
| import { useSidebarStore } from '@/stores/sidebar/store' | ||
|
|
||
| const logger = createLogger('Sidebar') | ||
|
|
||
| function isPlaceholderTask(id: string): boolean { | ||
| return id === 'new' || id.startsWith('draft-') | ||
| } | ||
|
|
||
| export function SidebarTooltip({ | ||
| children, | ||
| label, | ||
|
|
@@ -197,7 +203,7 @@ const SidebarTaskItem = memo(function SidebarTaskItem({ | |
| (isCurrentRoute || isSelected || isMenuOpen) && 'bg-[var(--surface-active)]' | ||
| )} | ||
| onClick={(e) => { | ||
| if (task.id === 'new') return | ||
| if (isPlaceholderTask(task.id)) return | ||
| if (e.metaKey || e.ctrlKey) return | ||
| if (e.shiftKey) { | ||
| e.preventDefault() | ||
|
|
@@ -209,14 +215,14 @@ const SidebarTaskItem = memo(function SidebarTaskItem({ | |
| }) | ||
| } | ||
| }} | ||
| onContextMenu={task.id !== 'new' ? (e) => onContextMenu(e, task.id) : undefined} | ||
| draggable={task.id !== 'new'} | ||
| onDragStart={task.id !== 'new' ? handleDragStart : undefined} | ||
| onDragEnd={task.id !== 'new' ? handleDragEnd : undefined} | ||
| onContextMenu={!isPlaceholderTask(task.id) ? (e) => onContextMenu(e, task.id) : undefined} | ||
| draggable={!isPlaceholderTask(task.id)} | ||
| onDragStart={!isPlaceholderTask(task.id) ? handleDragStart : undefined} | ||
| onDragEnd={!isPlaceholderTask(task.id) ? handleDragEnd : undefined} | ||
| > | ||
| <Blimp className='h-[16px] w-[16px] flex-shrink-0 text-[var(--text-icon)]' /> | ||
| <div className='min-w-0 flex-1 truncate font-base text-[var(--text-body)]'>{task.name}</div> | ||
| {task.id !== 'new' && ( | ||
| {!isPlaceholderTask(task.id) && ( | ||
| <div className='relative flex h-[18px] w-[18px] flex-shrink-0 items-center justify-center'> | ||
| {isActive && !isCurrentRoute && !isMenuOpen && ( | ||
| <span className='absolute h-[7px] w-[7px] animate-ping rounded-full bg-amber-400 opacity-30 group-hover:hidden' /> | ||
|
|
@@ -634,13 +640,19 @@ export const Sidebar = memo(function Sidebar() { | |
| [workspaces, workspaceId] | ||
| ) | ||
|
|
||
| const handleNewTaskFromNav = useCallback(() => { | ||
| flushSync(() => useDraftTaskStore.getState().createDraft()) | ||
| router.push(`/workspace/${workspaceId}/home`) | ||
| }, [router, workspaceId]) | ||
|
|
||
| const topNavItems = useMemo( | ||
| () => [ | ||
| { | ||
| id: 'home', | ||
| label: 'Home', | ||
| icon: Home, | ||
| href: `/workspace/${workspaceId}/home`, | ||
| onClick: handleNewTaskFromNav, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Home button now creates draft task on every clickMedium Severity The Home nav item now has Additional Locations (1)Reviewed by Cursor Bugbot for commit 2913b67. Configure here. |
||
| }, | ||
| { | ||
| id: 'search', | ||
|
|
@@ -649,7 +661,7 @@ export const Sidebar = memo(function Sidebar() { | |
| onClick: openSearchModal, | ||
| }, | ||
| ], | ||
| [workspaceId, openSearchModal] | ||
| [workspaceId, openSearchModal, handleNewTaskFromNav] | ||
| ) | ||
|
|
||
| const workspaceNavItems = useMemo( | ||
|
|
@@ -725,24 +737,53 @@ export const Sidebar = memo(function Sidebar() { | |
|
|
||
| useTaskEvents(workspaceId) | ||
|
|
||
| const tasks = useMemo( | ||
| () => | ||
| fetchedTasks.length > 0 | ||
| ? fetchedTasks.map((t) => ({ | ||
| ...t, | ||
| href: `/workspace/${workspaceId}/task/${t.id}`, | ||
| })) | ||
| : [ | ||
| { | ||
| id: 'new', | ||
| name: 'New task', | ||
| href: `/workspace/${workspaceId}/home`, | ||
| isActive: false, | ||
| isUnread: false, | ||
| }, | ||
| ], | ||
| [fetchedTasks, workspaceId] | ||
| ) | ||
| const draftTaskId = useDraftTaskStore((s) => s.draftTaskId) | ||
| const prevFetchedTaskIdsRef = useRef<Set<string>>(new Set(fetchedTasks.map((t) => t.id))) | ||
|
|
||
| useEffect(() => { | ||
| const currentIds = new Set(fetchedTasks.map((t) => t.id)) | ||
| if (draftTaskId) { | ||
| const hasNewTask = fetchedTasks.some((t) => !prevFetchedTaskIdsRef.current.has(t.id)) | ||
| if (hasNewTask) { | ||
| useDraftTaskStore.getState().removeDraft() | ||
| } | ||
| } | ||
| prevFetchedTaskIdsRef.current = currentIds | ||
| }, [draftTaskId, fetchedTasks]) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Draft removed prematurely on cold-start task loadMedium Severity
Reviewed by Cursor Bugbot for commit 2913b67. Configure here. |
||
|
|
||
| const tasks = useMemo(() => { | ||
| const mapped = fetchedTasks.map((t) => ({ | ||
| ...t, | ||
| href: `/workspace/${workspaceId}/task/${t.id}`, | ||
| })) | ||
|
|
||
| if (draftTaskId) { | ||
| const hasNewTask = fetchedTasks.some((t) => !prevFetchedTaskIdsRef.current.has(t.id)) | ||
| if (!hasNewTask) { | ||
| mapped.unshift({ | ||
| id: draftTaskId, | ||
| name: 'New task', | ||
| href: `/workspace/${workspaceId}/home`, | ||
| isActive: false, | ||
| isUnread: false, | ||
| updatedAt: new Date(), | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| if (mapped.length === 0) { | ||
| mapped.push({ | ||
| id: 'new', | ||
| name: 'New task', | ||
| href: `/workspace/${workspaceId}/home`, | ||
| isActive: false, | ||
| isUnread: false, | ||
| updatedAt: new Date(), | ||
| }) | ||
| } | ||
|
|
||
| return mapped | ||
| }, [fetchedTasks, workspaceId, draftTaskId]) | ||
|
|
||
| const { data: fetchedTables = [] } = useTablesList(workspaceId) | ||
| const { data: fetchedFiles = [] } = useWorkspaceFiles(workspaceId) | ||
|
|
@@ -784,7 +825,10 @@ export const Sidebar = memo(function Sidebar() { | |
| [fetchedKnowledgeBases, workspaceId, permissionConfig.hideKnowledgeBaseTab] | ||
| ) | ||
|
|
||
| const taskIds = useMemo(() => tasks.map((t) => t.id).filter((id) => id !== 'new'), [tasks]) | ||
| const taskIds = useMemo( | ||
| () => tasks.map((t) => t.id).filter((id) => !isPlaceholderTask(id)), | ||
| [tasks] | ||
| ) | ||
|
|
||
| const { selectedTasks, handleTaskClick } = useTaskSelection({ taskIds }) | ||
|
|
||
|
|
@@ -1088,9 +1132,12 @@ export const Sidebar = memo(function Sidebar() { | |
| const tasksPrimaryAction = useMemo( | ||
| () => ({ | ||
| label: 'New task', | ||
| onSelect: () => navigateToPage(`/workspace/${workspaceId}/home`), | ||
| onSelect: () => { | ||
| flushSync(() => useDraftTaskStore.getState().createDraft()) | ||
| router.push(`/workspace/${workspaceId}/home`) | ||
| }, | ||
| }), | ||
| [navigateToPage, workspaceId] | ||
| [router, workspaceId] | ||
| ) | ||
|
|
||
| const workflowsPrimaryAction = useMemo( | ||
|
|
@@ -1109,10 +1156,10 @@ export const Sidebar = memo(function Sidebar() { | |
| [toggleCollapsed] | ||
| ) | ||
|
|
||
| const handleNewTask = useCallback( | ||
| () => navigateToPage(`/workspace/${workspaceId}/home`), | ||
| [navigateToPage, workspaceId] | ||
| ) | ||
| const handleNewTask = useCallback(() => { | ||
| flushSync(() => useDraftTaskStore.getState().createDraft()) | ||
| router.push(`/workspace/${workspaceId}/home`) | ||
| }, [router, workspaceId]) | ||
|
|
||
| const handleSeeMoreTasks = useCallback(() => setVisibleTaskCount((prev) => prev + 5), []) | ||
|
|
||
|
|
@@ -1453,7 +1500,8 @@ export const Sidebar = memo(function Sidebar() { | |
| {tasks.slice(0, visibleTaskCount).map((task) => { | ||
| const isCurrentRoute = task.id !== 'new' && pathname === task.href | ||
| const isRenaming = taskFlyoutRename.editingId === task.id | ||
| const isSelected = task.id !== 'new' && selectedTasks.has(task.id) | ||
| const isSelected = | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Collapsed sidebar allows actions on draft placeholder tasksMedium Severity The sidebar isn't consistently recognizing placeholder tasks (like new drafts). This means action buttons and context menus are incorrectly available for these tasks in the collapsed sidebar, which could lead to attempts at server-side operations on non-existent tasks. Also, the Reviewed by Cursor Bugbot for commit 2913b67. Configure here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isCurrentRoute check not updated for draft task IDsLow Severity Both the collapsed (line 1478) and expanded (line 1501) sidebar compute Additional Locations (1)Reviewed by Cursor Bugbot for commit 2913b67. Configure here. |
||
| !isPlaceholderTask(task.id) && selectedTasks.has(task.id) | ||
|
|
||
| if (isRenaming) { | ||
| return ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { create } from 'zustand' | ||
| import { devtools } from 'zustand/middleware' | ||
| import { generateShortId } from '@/lib/core/utils/uuid' | ||
|
|
||
| interface DraftTaskState { | ||
| /** ID of the current draft task, or null if none exists */ | ||
| draftTaskId: string | null | ||
| /** Creates a draft task (reuses existing if one exists). Returns the draft ID. */ | ||
| createDraft: () => string | ||
| /** Removes the current draft task */ | ||
| removeDraft: () => void | ||
| } | ||
|
|
||
| export const useDraftTaskStore = create<DraftTaskState>()( | ||
| devtools( | ||
| (set, get) => ({ | ||
| draftTaskId: null, | ||
|
|
||
| createDraft: () => { | ||
| const existing = get().draftTaskId | ||
| if (existing) return existing | ||
| const id = `draft-${generateShortId(8)}` | ||
| set({ draftTaskId: id }) | ||
| return id | ||
| }, | ||
|
|
||
| removeDraft: () => set({ draftTaskId: null }), | ||
| }), | ||
| { name: 'draft-task-store' } | ||
| ) | ||
| ) |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Three identical draft-creation handlers are redundant
Low Severity
handleNewTaskFromNav,tasksPrimaryAction.onSelect, andhandleNewTaskall contain identical logic: callingflushSyncwithcreateDraft()thenrouter.pushto home. Two of these areuseCallbackwrappers with the same deps.tasksPrimaryAction.onSelectcould referencehandleNewTask, andhandleNewTaskFromNavcould be eliminated entirely in favor ofhandleNewTask, reducing the maintenance surface for this logic from three copies to one.Additional Locations (2)
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx#L1131-L1141apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx#L1158-L1162Reviewed by Cursor Bugbot for commit 2913b67. Configure here.