feat(desktop): Add task detail view with Tiptap editor#977
feat(desktop): Add task detail view with Tiptap editor#977saddlepaddle merged 10 commits intomainfrom
Conversation
- Add full-page task detail view at /tasks/$taskId (Linear-style UI) - Create API proxy for Linear images (uploads.linear.app requires auth) - Update SafeImage to route Linear URLs through proxy - Update CSP to allow images from API URL - Co-locate TasksView components under tasks route
📝 WalkthroughWalkthroughThis PR adds a complete task detail viewing and editing feature to the desktop application. It introduces a Linear image proxy API endpoint with authentication, updates Content Security Policy headers, adds task detail page routing with in-place editing of titles and descriptions, implements a rich TipTap-based markdown editor with syntax highlighting and slash commands, and adds task property management dropdowns for status, priority, and assignees alongside activity tracking and comment functionality. Changes
Sequence DiagramsequenceDiagram
participant User
participant TaskList as Task List Page
participant TaskDetail as Task Detail Page
participant Collections as Collections/Database
participant MarkdownEditor as Markdown Editor
User->>TaskList: Click task row
TaskList->>TaskDetail: Navigate to /tasks/{taskId}
TaskDetail->>Collections: Fetch task (useLiveQuery)
Collections-->>TaskDetail: Return TaskWithStatus
TaskDetail->>User: Render editable title, description, properties
User->>TaskDetail: Edit title
TaskDetail->>TaskDetail: Local state update
User->>TaskDetail: Blur/Enter
TaskDetail->>Collections: collections.tasks.update(title)
Collections-->>TaskDetail: Confirm update
User->>MarkdownEditor: Edit description (in TipTap)
MarkdownEditor->>MarkdownEditor: Parse markdown extensions
User->>MarkdownEditor: Blur editor
MarkdownEditor->>TaskDetail: onSave(markdown)
TaskDetail->>Collections: collections.tasks.update(description)
Collections-->>TaskDetail: Confirm update
User->>TaskDetail: Open Status/Priority/Assignee dropdown
TaskDetail->>Collections: Fetch statuses/users (useLiveQuery)
Collections-->>TaskDetail: Return list
TaskDetail->>User: Render dropdown menu
User->>TaskDetail: Select option
TaskDetail->>Collections: collections.tasks.update(property)
Collections-->>TaskDetail: Confirm update
TaskDetail->>User: Close dropdown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/api/src/app/api/proxy/linear-image/route.ts`:
- Around line 56-72: Add a timeout to the external fetch to Linear by passing an
AbortSignal.timeout (use a constant like FETCH_TIMEOUT_MS) to the fetch call
that requests linearUrl with connection.accessToken, and wrap the fetch in a
try/catch to handle a timeout specifically; on timeout log with
"[proxy/linear-image] Fetch timeout" including the URL and return a 504
Response, while other errors should be rethrown or handled as currently done for
linearResponse.ok. Ensure you reference the existing linearUrl,
connection.accessToken, and linearResponse handling flow when inserting the
timeout and error handling.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskDetailView/TaskDetailView.tsx:
- Around line 77-81: Remove the type assertion on task.status.type before
passing it to StatusIcon and add a validation/translation layer (e.g., in the
TaskDetailView component or the API response handler) that maps incoming
database status strings (task.status.type) to the frontend's allowed StatusType
values or rejects/normalizes unknown values; update uses of StatusIcon and
StatusType so you validate/map task.status.type to a safe StatusType (or a
defined fallback like "unstarted"/"started") and ensure progress uses
task.status.progressPercent ?? undefined to preserve previous behavior.
🧹 Nitpick comments (6)
apps/api/src/app/api/proxy/linear-image/route.ts (1)
23-28: Consider adding Zod schema validation for query parameters.Per coding guidelines for API routes, input validation should use Zod schemas at boundaries. While the manual validation works, a schema would provide better type safety and consistent error messages.
♻️ Suggested refactor with Zod validation
import { auth } from "@superset/auth/server"; import { db } from "@superset/db/client"; import { integrationConnections } from "@superset/db/schema"; import { and, eq } from "drizzle-orm"; +import { z } from "zod"; const LINEAR_IMAGE_HOST = "uploads.linear.app"; const CACHE_MAX_AGE = 31536000; // 1 year (Linear URLs are content-addressed) +const querySchema = z.object({ + url: z.string().url(), +}); + export async function GET(request: Request): Promise<Response> { // ... auth checks ... const url = new URL(request.url); - const linearUrl = url.searchParams.get("url"); - - if (!linearUrl) { - return new Response("Missing url parameter", { status: 400 }); - } + const parsed = querySchema.safeParse({ + url: url.searchParams.get("url"), + }); + + if (!parsed.success) { + return new Response("Invalid or missing url parameter", { status: 400 }); + } + + const linearUrl = parsed.data.url;apps/desktop/src/renderer/components/MarkdownRenderer/components/SafeImage/SafeImage.tsx (1)
4-4: Consider extracting the Linear host constant to a shared package.
LINEAR_IMAGE_HOSTis duplicated between this file andapps/api/src/app/api/proxy/linear-image/route.ts. If this value ever changes, both locations need to be updated.This is a minor concern since the value is unlikely to change, but for maintainability you could extract it to a shared constants package if one exists in the monorepo.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/page.tsx (2)
44-46: Consider memoizinghandleBackfor referential stability.While not critical since it's passed to a single child, wrapping in
useCallbackis a common best practice to prevent unnecessary re-renders ifTaskDetailViewis memoized.♻️ Optional improvement
+import { useCallback, useMemo } from "react"; -import { useMemo } from "react"; - const handleBack = () => { - navigate({ to: "/tasks" }); - }; + const handleBack = useCallback(() => { + navigate({ to: "/tasks" }); + }, [navigate]);
39-54: Extract theisLoadingstate fromuseLiveQueryto distinguish loading from "not found".The code currently shows "Task not found" both during data loading and when the task genuinely doesn't exist.
useLiveQueryprovides anisLoadingstate (available alongsidedata) that can be used to render a loading state instead of the error message while data is fetching.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskDetailView/TaskDetailView.tsx (2)
29-36: Add an accessible label to the back button.Screen readers won't understand the purpose of this icon-only button without an accessible name.
♻️ Proposed fix
<Button variant="ghost" size="icon" className="h-8 w-8" onClick={onBack} + aria-label="Back to tasks" > <HiArrowLeft className="w-4 h-4" /> </Button>
102-107: UseSafeImagefor assignee avatar.SafeImage handles Linear image proxying with authenticated requests. Since assignee images come from the users collection and can originate from Linear, direct
<img>tags will fail to load due to missing authentication. SafeImage automatically detects Linear URLs and proxies them with credentials.♻️ Proposed change
+import { SafeImage } from "renderer/components/MarkdownRenderer/components/SafeImage"; - <img - src={task.assignee.image} - alt="" - className="w-5 h-5 rounded-full" - /> + <SafeImage + src={task.assignee.image} + alt="" + className="w-5 h-5 rounded-full" + />
| // Fetch the image from Linear with auth | ||
| const linearResponse = await fetch(linearUrl, { | ||
| headers: { | ||
| Authorization: `Bearer ${connection.accessToken}`, | ||
| }, | ||
| }); | ||
|
|
||
| if (!linearResponse.ok) { | ||
| console.error("[proxy/linear-image] Linear fetch failed:", { | ||
| status: linearResponse.status, | ||
| statusText: linearResponse.statusText, | ||
| url: linearUrl, | ||
| }); | ||
| return new Response("Failed to fetch image from Linear", { | ||
| status: linearResponse.status, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add timeout to external fetch to prevent hanging requests.
The fetch to Linear has no timeout, which could cause the request to hang indefinitely if Linear is slow or unresponsive. This is a reliability concern per coding guidelines on external-call hazards.
🛡️ Proposed fix with timeout
+const FETCH_TIMEOUT_MS = 30000; // 30 seconds
+
// Fetch the image from Linear with auth
const linearResponse = await fetch(linearUrl, {
headers: {
Authorization: `Bearer ${connection.accessToken}`,
},
+ signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
});You may also want to catch the timeout error specifically:
try {
const linearResponse = await fetch(linearUrl, {
headers: {
Authorization: `Bearer ${connection.accessToken}`,
},
signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
});
// ... rest of handling
} catch (error) {
if (error instanceof Error && error.name === "TimeoutError") {
console.error("[proxy/linear-image] Fetch timeout:", { url: linearUrl });
return new Response("Request to Linear timed out", { status: 504 });
}
throw error;
}📝 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.
| // Fetch the image from Linear with auth | |
| const linearResponse = await fetch(linearUrl, { | |
| headers: { | |
| Authorization: `Bearer ${connection.accessToken}`, | |
| }, | |
| }); | |
| if (!linearResponse.ok) { | |
| console.error("[proxy/linear-image] Linear fetch failed:", { | |
| status: linearResponse.status, | |
| statusText: linearResponse.statusText, | |
| url: linearUrl, | |
| }); | |
| return new Response("Failed to fetch image from Linear", { | |
| status: linearResponse.status, | |
| }); | |
| } | |
| const FETCH_TIMEOUT_MS = 30000; // 30 seconds | |
| // Fetch the image from Linear with auth | |
| const linearResponse = await fetch(linearUrl, { | |
| headers: { | |
| Authorization: `Bearer ${connection.accessToken}`, | |
| }, | |
| signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), | |
| }); | |
| if (!linearResponse.ok) { | |
| console.error("[proxy/linear-image] Linear fetch failed:", { | |
| status: linearResponse.status, | |
| statusText: linearResponse.statusText, | |
| url: linearUrl, | |
| }); | |
| return new Response("Failed to fetch image from Linear", { | |
| status: linearResponse.status, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@apps/api/src/app/api/proxy/linear-image/route.ts` around lines 56 - 72, Add a
timeout to the external fetch to Linear by passing an AbortSignal.timeout (use a
constant like FETCH_TIMEOUT_MS) to the fetch call that requests linearUrl with
connection.accessToken, and wrap the fetch in a try/catch to handle a timeout
specifically; on timeout log with "[proxy/linear-image] Fetch timeout" including
the URL and return a 504 Response, while other errors should be rethrown or
handled as currently done for linearResponse.ok. Ensure you reference the
existing linearUrl, connection.accessToken, and linearResponse handling flow
when inserting the timeout and error handling.
| <StatusIcon | ||
| type={task.status.type as StatusType} | ||
| color={task.status.color} | ||
| progress={task.status.progressPercent ?? undefined} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find status type definitions and usages
rg -n "type.*StatusType|statusType|status\.type" --type=ts -g '!node_modules' -C 2Repository: superset-sh/superset
Length of output: 22922
🏁 Script executed:
#!/bin/bash
# Find the TaskDetailView file and inspect it
find . -name "TaskDetailView.tsx" -type f | head -5Repository: superset-sh/superset
Length of output: 183
🏁 Script executed:
#!/bin/bash
# Search for StatusType enum or type definition
rg -n "enum StatusType|type StatusType|interface StatusType" --type=ts -g '!node_modules'Repository: superset-sh/superset
Length of output: 1040
🏁 Script executed:
#!/bin/bash
# Find task interface/type definitions
rg -n "interface.*[Tt]ask|type.*[Tt]ask.*=" --type=ts -g '!node_modules' | grep -i task | head -20Repository: superset-sh/superset
Length of output: 2496
🏁 Script executed:
#!/bin/bash
# Inspect TaskWithStatus definition in full
sed -n '35,50p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsxRepository: superset-sh/superset
Length of output: 392
🏁 Script executed:
#!/bin/bash
# Check SelectTaskStatus schema to verify status.type definition
rg -n "SelectTaskStatus|status.*type" packages/db/src/schema/schema.ts -A 3 -B 1 | head -40Repository: superset-sh/superset
Length of output: 494
🏁 Script executed:
#!/bin/bash
# Find taskStatuses table definition and the enum for type field
rg -n "taskStatusEnum|taskStatuses.*=" packages/db/src/schema/ -A 5 -B 1 | head -50Repository: superset-sh/superset
Length of output: 2537
🏁 Script executed:
#!/bin/bash
# Check the full taskStatuses table definition
rg -n "pgTable.*taskStatuses|export const taskStatuses" packages/db/src/schema/schema.ts -A 20Repository: superset-sh/superset
Length of output: 799
🏁 Script executed:
#!/bin/bash
# Check what taskStatusEnumValues actually contains
cat packages/db/src/schema/enums.tsRepository: superset-sh/superset
Length of output: 1242
🏁 Script executed:
#!/bin/bash
# Search for where task status data is validated from API
rg -n "taskStatusEnum|TaskStatus" packages/db/src/schema/ -B 2 -A 2Repository: superset-sh/superset
Length of output: 1752
🏁 Script executed:
#!/bin/bash
# Check if there's a mapping layer between TaskStatus and StatusType
rg -n "TaskStatus|StatusType" apps/desktop/src/renderer --type=ts -B 2 -A 2 | grep -A 5 -B 5 "map\|transform\|convert" | head -40Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check how TaskWithStatus is used to see if data is transformed
rg -n "TaskWithStatus" apps/desktop/src/renderer --type=ts -B 1 -A 1 | head -30Repository: superset-sh/superset
Length of output: 3654
🏁 Script executed:
#!/bin/bash
# Search for where task data comes from (API/database query)
rg -n "trpc.*task|useQuery" apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks --type=ts -B 1 -A 3 | head -50Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the actual schema.ts to see how taskStatuses.type is defined
sed -n '64,90p' packages/db/src/schema/schema.tsRepository: superset-sh/superset
Length of output: 925
🏁 Script executed:
#!/bin/bash
# Check if there's validation happening at the API/tRPC layer
find apps/api -name "*.ts" -type f | xargs rg -n "taskStatuses.*type|status\.type" -B 2 -A 2 | head -60Repository: superset-sh/superset
Length of output: 1641
Remove assertion and add proper type validation layer for status types.
The assertion masks a data mismatch: the database schema permits 8 status type values ("backlog", "todo", "planning", "working", "needs-feedback", "ready-to-merge", "completed", "canceled"), but StatusType only accepts 5 ("backlog", "unstarted", "started", "completed", "canceled"). If status data with unrecognized values reaches StatusIcon, the assertion bypasses all type checking.
Either narrow the database schema to match the frontend's valid values, or add a validation/transformation layer at the API boundary to ensure only expected values are transmitted to the renderer.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskDetailView/TaskDetailView.tsx
around lines 77 - 81, Remove the type assertion on task.status.type before
passing it to StatusIcon and add a validation/translation layer (e.g., in the
TaskDetailView component or the API response handler) that maps incoming
database status strings (task.status.type) to the frontend's allowed StatusType
values or rejects/normalizes unknown values; update uses of StatusIcon and
StatusType so you validate/map task.status.type to a safe StatusType (or a
defined fallback like "unstarted"/"started") and ensure progress uses
task.status.progressPercent ?? undefined to preserve previous behavior.
- Format useEscapeToNavigate conditional - Remove !important from CSS (use specificity instead)
- Create EditableTitle component - Save on blur, Enter commits, Escape reverts - Styled to match existing h1 appearance
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/ActivitySection/ActivitySection.tsx:
- Line 18: The avatarFallback currently uses creatorName.charAt(0).toUpperCase()
which produces an empty string when creatorName is empty; update the
ActivitySection component to guard against empty or undefined creatorName (in
the expression used for avatarFallback) and provide a sensible default character
(e.g., '?' or 'U') so the fallback always renders a visible character; locate
the avatarFallback prop usage in ActivitySection.tsx and replace the direct
charAt call with a safe expression that checks creatorName and falls back to the
default.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/CommentInput/CommentInput.tsx:
- Around line 1-12: The current CommentInput component renders a non-interactive
div which looks like an input but is not keyboard/screen-reader accessible;
replace the div with a semantic control (e.g., a <textarea>) inside the
CommentInput function so users can type and navigate via keyboard and AT. Update
the component signature (CommentInput) to accept/forward value and onChange (or
an onSubmit) props, add an accessible label/aria-label, appropriate
rows/placeholder handling, and keep the existing styling/className on the
textarea; alternatively, if the component is only a visual placeholder, rename
it to avoid implying interactivity (e.g., CommentPlaceholder) and keep it
non-interactive.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/PriorityProperty/PriorityProperty.tsx:
- Around line 39-47: The try/catch currently wraps collections.tasks.update(...)
but does not await the async work performed by the ElectricSQL onUpdate callback
(which calls apiClient.task.update.mutate()), so API errors are not caught and
setOpen(false) runs too soon; fix by ensuring the update call is awaited or by
using a promise-aware mechanism: modify the code around collections.tasks.update
in PriorityProperty.tsx to either call an async variant or wrap the onUpdate
flow so you can await completion (or return a Promise from the update) before
calling setOpen(false), and move the try/catch to surround the awaited operation
so errors from apiClient.task.update.mutate() are caught and handled.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/StatusProperty/StatusProperty.tsx:
- Around line 35-37: sortedStatuses is created by memoizing
statuses.sort(compareStatusesForDropdown) which mutates the original statuses
array; update the useMemo in StatusProperty (the sortedStatuses computation) to
first copy the array (e.g., via statuses.slice() or [...statuses]) and then call
sort(compareStatusesForDropdown) on the copy so the original statuses array is
not mutated.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx:
- Line 12: The copy-to-clipboard flow in CodeBlockView currently uses
navigator.clipboard.writeText without handling rejection and uses a magic
timeout; update the CodeBlockView component to (1) introduce a named constant
like CLIPBOARD_FEEDBACK_DURATION_MS for the timeout instead of the literal
number, (2) wrap navigator.clipboard.writeText(...) in a try/catch or promise
.catch and on error call console.error or processLogger.error with a prefixed
message (e.g., "CodeBlockView: clipboard write failed:") plus the error, and (3)
ensure success/failure feedback still uses CLIPBOARD_FEEDBACK_DURATION_MS; also
keep LANGUAGES as-is but ensure any related UI toggles rely on the new constant.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskMarkdownRenderer/components/LinearImage/LinearImage.tsx:
- Around line 8-14: The isLinearImageUrl function currently swallows URL parse
errors; update its catch block to log the malformed src and the caught error
(e.g., include src and error.message) before returning false so failures are
observable; modify the catch in isLinearImageUrl to call the app's logger (or
console as a fallback) with context including LINEAR_IMAGE_HOST and the input
src, then return false, leaving the rest of the URL parsing logic unchanged.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/page.tsx:
- Around line 50-72: The component currently treats a falsy taskData as "Task
not found"; instead, read the loading state from your data hook (e.g., the
isLoading/ready flag returned by useLiveQuery or whichever hook provides it) and
check that before showing the not-found UI. Update the logic where task is
derived (task, taskData) and the early return that renders "Task not found" so
it first returns a loading placeholder while isLoading is true, and only shows
the not-found message when isLoading is false and task is null; keep existing
handlers like handleSaveDescription and handleBack unchanged.
🧹 Nitpick comments (6)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/LinearImage/LinearImage.tsx (1)
41-49: Avoid double URL parsing.Compute
isLinearonce to prevent duplicate parsing (and duplicate logs if you add logging).♻️ Proposed refactor
- const proxiedSrc = isLinearImageUrl(src) ? getLinearProxyUrl(src) : src; + const isLinear = isLinearImageUrl(src); + const proxiedSrc = isLinear ? getLinearProxyUrl(src) : src; ... - crossOrigin={isLinearImageUrl(src) ? "use-credentials" : undefined} + crossOrigin={isLinear ? "use-credentials" : undefined}apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/TaskMarkdownRenderer.tsx (2)
37-50: Code duplication with LinearImage component.These utility functions (
isLinearImageUrl,getLinearProxyUrl) are duplicated inLinearImage.tsx(see relevant code snippets, lines 35-51). Consider extracting them to a shared utility module to maintain a single source of truth.♻️ Suggested refactor
Create a shared utility file:
// utils/linear-image.ts const LINEAR_IMAGE_HOST = "uploads.linear.app"; export function isLinearImageUrl(src: string): boolean { try { const url = new URL(src); return url.host === LINEAR_IMAGE_HOST; } catch { return false; } } export function getLinearProxyUrl(linearUrl: string): string { const proxyUrl = new URL(`${env.NEXT_PUBLIC_API_URL}/api/proxy/linear-image`); proxyUrl.searchParams.set("url", linearUrl); return proxyUrl.toString(); }Then import from both
TaskMarkdownRenderer.tsxandLinearImage.tsx.
216-224: Unsafe type assertion on editor storage.The type assertion
editor.storage as unknown as Record<string, { getMarkdown?: () => string }>bypasses TypeScript's type checking. Thetiptap-markdownextension's storage typing should be used if available, or the type should be narrowed more safely.♻️ Suggested approach
Consider a safer approach with runtime validation:
onBlur: ({ editor }) => { - const storage = editor.storage as unknown as Record< - string, - { getMarkdown?: () => string } - >; - const markdown = storage.markdown?.getMarkdown?.() ?? ""; + const markdownStorage = editor.storage.markdown as + | { getMarkdown?: () => string } + | undefined; + const markdown = + typeof markdownStorage?.getMarkdown === "function" + ? markdownStorage.getMarkdown() + : ""; onSave(markdown); },apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/SlashCommand/SlashCommand.tsx (1)
169-197: Minor inefficiency in flat index lookup.The
find()on line 173 performs an O(n) lookup for each rendered item. For the current small command set (~8 items), this is negligible. If the command list grows significantly, consider using aMapfor O(1) lookups.♻️ Optional optimization
+ // Build a Map for O(1) index lookups + const indexByTitle = new Map( + itemsWithIndex.map((item) => [item.title, item.flatIndex]), + ); + return ( <div ref={containerRef} className="bg-popover text-popover-foreground rounded-md border p-1 shadow-md overflow-hidden max-h-80 overflow-y-auto w-48" > {groups.map((group, groupIndex) => ( <div key={group.key}> {groupIndex > 0 && <div className="bg-border -mx-1 my-1 h-px" />} {group.items.map((item) => { - const itemWithIndex = itemsWithIndex.find( - (i) => i.title === item.title, - ); - const index = itemWithIndex?.flatIndex ?? 0; + const index = indexByTitle.get(item.title) ?? 0;apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/PriorityProperty/PriorityProperty.tsx (1)
44-46: Error is logged but user receives no feedback.The user won't know the priority update failed. Consider adding a toast notification or visual error state.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/StatusProperty/StatusProperty.tsx (1)
45-52: Same concerns as PriorityProperty: async handling and user feedback.If
collections.tasks.updateis async, the catch won't handle rejections. Additionally, the user receives no feedback on failure.
| <div className="space-y-3"> | ||
| <ActivityItem | ||
| avatarUrl={creatorAvatarUrl} | ||
| avatarFallback={creatorName.charAt(0).toUpperCase()} |
There was a problem hiding this comment.
Guard against empty creatorName.
If creatorName is an empty string, charAt(0) returns "", which would render an empty fallback avatar. Consider adding a fallback character.
Proposed fix
- avatarFallback={creatorName.charAt(0).toUpperCase()}
+ avatarFallback={creatorName.charAt(0)?.toUpperCase() || "?"}📝 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.
| avatarFallback={creatorName.charAt(0).toUpperCase()} | |
| avatarFallback={creatorName.charAt(0)?.toUpperCase() || "?"} |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/ActivitySection/ActivitySection.tsx
at line 18, The avatarFallback currently uses
creatorName.charAt(0).toUpperCase() which produces an empty string when
creatorName is empty; update the ActivitySection component to guard against
empty or undefined creatorName (in the expression used for avatarFallback) and
provide a sensible default character (e.g., '?' or 'U') so the fallback always
renders a visible character; locate the avatarFallback prop usage in
ActivitySection.tsx and replace the direct charAt call with a safe expression
that checks creatorName and falls back to the default.
| interface CommentInputProps { | ||
| placeholder?: string; | ||
| } | ||
|
|
||
| export function CommentInput({ | ||
| placeholder = "Leave a comment...", | ||
| }: CommentInputProps) { | ||
| return ( | ||
| <div className="border border-border rounded-lg p-3 text-sm text-muted-foreground cursor-text hover:border-muted-foreground/50 transition-colors"> | ||
| {placeholder} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
CommentInput is visually an input but not actually interactive.
If this is meant to accept comments, use a semantic control (e.g., <textarea>) so keyboard users and screen readers can interact with it. If it’s only a placeholder, consider renaming to avoid confusion.
🛠️ Suggested direction (semantic input)
- <div className="border border-border rounded-lg p-3 text-sm text-muted-foreground cursor-text hover:border-muted-foreground/50 transition-colors">
- {placeholder}
- </div>
+ <textarea
+ className="border border-border rounded-lg p-3 text-sm text-muted-foreground cursor-text hover:border-muted-foreground/50 transition-colors w-full resize-none"
+ placeholder={placeholder}
+ rows={1}
+ />🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/CommentInput/CommentInput.tsx
around lines 1 - 12, The current CommentInput component renders a
non-interactive div which looks like an input but is not keyboard/screen-reader
accessible; replace the div with a semantic control (e.g., a <textarea>) inside
the CommentInput function so users can type and navigate via keyboard and AT.
Update the component signature (CommentInput) to accept/forward value and
onChange (or an onSubmit) props, add an accessible label/aria-label, appropriate
rows/placeholder handling, and keep the existing styling/className on the
textarea; alternatively, if the component is only a visual placeholder, rename
it to avoid implying interactivity (e.g., CommentPlaceholder) and keep it
non-interactive.
| try { | ||
| collections.tasks.update(task.id, (draft) => { | ||
| draft.priority = newPriority; | ||
| }); | ||
| setOpen(false); | ||
| } catch (error) { | ||
| console.error("[PriorityProperty] Failed to update priority:", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the collections implementation
fd -e ts -e tsx -path "*/collections*" | head -20Repository: superset-sh/superset
Length of output: 232
🏁 Script executed:
# Search for where collections is imported/defined in the file
rg -n "collections" apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/\$taskId/components/PropertiesSidebar/components/PriorityProperty/PriorityProperty.tsx -B 5 -A 2Repository: superset-sh/superset
Length of output: 458
🏁 Script executed:
# Search for collections definition - likely a hook or context
rg -n "useCollections|export.*collections" --type ts -g "**/*.ts" -g "**/*.tsx" | grep -E "useCollections|const.*collections|export" | head -20Repository: superset-sh/superset
Length of output: 3454
🏁 Script executed:
# Look for the update method definition
rg -n "\.update\s*[=(]" --type ts --type tsx -A 5 | grep -E "update|async|Promise" | head -30Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Find CollectionsProvider for desktop
find . -name "CollectionsProvider.tsx" -path "*/desktop/*"Repository: superset-sh/superset
Length of output: 167
🏁 Script executed:
# Get the content of the renderer CollectionsProvider
cat apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider.tsxRepository: superset-sh/superset
Length of output: 177
🏁 Script executed:
# Get the content of the renderer CollectionsProvider with correct path
cat apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/CollectionsProvider.tsxRepository: superset-sh/superset
Length of output: 1239
🏁 Script executed:
# Find the collections.ts file
find . -name "collections.ts" -path "*/desktop/*"Repository: superset-sh/superset
Length of output: 158
🏁 Script executed:
# Get the content of collections.ts
cat apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsRepository: superset-sh/superset
Length of output: 7876
🏁 Script executed:
# Search for how collections.tasks.update is used in the codebase
rg "collections\.tasks\.update" --type ts --type tsx -B 2 -A 5Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Also search for .update calls on collections to see the pattern
rg "collections\.\w+\.update\(" --type ts --type tsx -B 2 -A 5 | head -80Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for collections.tasks.update usage
rg "collections\.tasks\.update" -B 2 -A 5Repository: superset-sh/superset
Length of output: 14977
🏁 Script executed:
# Search for .update calls on collections
rg "collections\.\w+\.update\(" -B 2 -A 5 | head -100Repository: superset-sh/superset
Length of output: 15106
🏁 Script executed:
# Look for the update method signature in TanStack react-db or electric-db
rg "\.update\s*\(" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts -B 2 -A 10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there are any awaits on collections.update calls anywhere
rg "await\s+collections\.\w+\.update"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for the Collection type and update method definition
rg "export.*Collection|interface Collection" --type ts | head -20Repository: superset-sh/superset
Length of output: 1191
🏁 Script executed:
# Check TanStack react-db Collection type - look in node_modules or type definitions
find . -name "*.d.ts" -o -name "package.json" | grep -E "tanstack.*react.*db|electric.*db" | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at the onUpdate handler to understand the async flow
cat apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | grep -A 15 "onUpdate:"Repository: superset-sh/superset
Length of output: 1525
🏁 Script executed:
# Check if Collections type is imported and where it comes from
rg "type Collections" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/Repository: superset-sh/superset
Length of output: 219
🏁 Script executed:
# Search for `@tanstack/react-db` in package.json to understand the library version
rg "@tanstack/react-db" package.jsonRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for any documentation or type definitions that mention Collection.update return type
find . -name "node_modules" -prune -o -type f -name "*.ts" -print | xargs grep -l "Collection.*update" | head -5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's any Promise handling in other parts of the codebase for collection updates
rg "\.update.*\?" apps/desktop/src/renderer -B 2 -A 2 | head -40Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for any return value usage of collections.tasks.update
rg "collections\.tasks\.update" -A 3 | grep -E "await|then|Promise" | head -20Repository: superset-sh/superset
Length of output: 3250
🏁 Script executed:
# Check if update is wrapped in try-catch consistently across the codebase
rg "try \{" -A 5 apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks | grep -E "try|catch|update" | head -30Repository: superset-sh/superset
Length of output: 5092
🏁 Script executed:
# Look at TanStack electric-db-collection imports and usage
rg "electricCollectionOptions|Collection" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | head -10Repository: superset-sh/superset
Length of output: 511
🏁 Script executed:
# Search for documentation or comments about how update works
rg "update.*synchronous|update.*async|update.*Promise" apps/desktop/src/renderer -B 2 -A 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for imports of Collection from react-db to understand its interface
rg "from.*react-db|from.*electric-db-collection" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/Repository: superset-sh/superset
Length of output: 529
🏁 Script executed:
# Check package.json for `@tanstack/react-db` version
cat package.json | grep -A 2 -B 2 "@tanstack/react-db"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Find and check package.json files
find . -name "package.json" -not -path "*/node_modules/*" | xargs grep -l "@tanstack/react-db"Repository: superset-sh/superset
Length of output: 118
🏁 Script executed:
# Check the root package.json and any workspace package.json files
cat package.json | grep "@tanstack"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check desktop package.json for `@tanstack` dependencies
cat apps/desktop/package.json | grep -E "@tanstack|electric"Repository: superset-sh/superset
Length of output: 484
🏁 Script executed:
# Check if there are any TypeScript errors or comments about async handling in the codebase
rg "Promise|async|await" apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks -B 1 -A 1 | grep -E "update|Promise" | head -20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at the actual file to see the complete context of the handleSelectPriority function
cat -n apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/\$taskId/components/PropertiesSidebar/components/PriorityProperty/PriorityProperty.tsx | sed -n '30,50p'Repository: superset-sh/superset
Length of output: 711
🏁 Script executed:
# Search for any TypeScript type checking or JSDoc about update method
rg "@param|@returns|update.*void|update.*Promise" apps/desktop/src/renderer -B 2 -A 2 | head -30Repository: superset-sh/superset
Length of output: 3364
🏁 Script executed:
# Check if there's any way onUpdate errors propagate back to the caller
rg "onUpdate|onInsert|onDelete" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts -B 3 -A 10 | head -50Repository: superset-sh/superset
Length of output: 1511
🏁 Script executed:
# Look for return type of update method in comments or actual usage patterns
rg "collections\.\w+\.update\(" apps/desktop/src/renderer -B 5 | grep -E "const|let|await|return|void" | head -20Repository: superset-sh/superset
Length of output: 2959
🏁 Script executed:
# Check if there's any error handling at the Collection level for onUpdate callbacks
rg "onError|catch.*onUpdate|\.catch\(" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for similar patterns in the mobile app to compare
cat apps/mobile/lib/collections/collections.ts | grep -A 20 "onUpdate:" | head -30Repository: superset-sh/superset
Length of output: 906
Async errors from collections.tasks.update won't be caught by the try/catch block.
The collections.tasks.update() method itself may be synchronous, but the onUpdate callback registered in ElectricSQL is async and makes API calls to apiClient.task.update.mutate(). This means API failures won't be caught by the current error handling, and setOpen(false) executes before the update completes, allowing the dropdown to close while the operation is still pending.
Either await the update or handle errors differently if the library API doesn't return a Promise.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/PriorityProperty/PriorityProperty.tsx
around lines 39 - 47, The try/catch currently wraps
collections.tasks.update(...) but does not await the async work performed by the
ElectricSQL onUpdate callback (which calls apiClient.task.update.mutate()), so
API errors are not caught and setOpen(false) runs too soon; fix by ensuring the
update call is awaited or by using a promise-aware mechanism: modify the code
around collections.tasks.update in PriorityProperty.tsx to either call an async
variant or wrap the onUpdate flow so you can await completion (or return a
Promise from the update) before calling setOpen(false), and move the try/catch
to surround the awaited operation so errors from apiClient.task.update.mutate()
are caught and handled.
| const sortedStatuses = useMemo(() => { | ||
| return statuses.sort(compareStatusesForDropdown); | ||
| }, [statuses]); |
There was a problem hiding this comment.
Array mutation bug: sort() mutates in place.
statuses.sort() mutates the original array, which can cause unpredictable behavior since statuses comes from useMemo. Create a copy before sorting.
Proposed fix
const sortedStatuses = useMemo(() => {
- return statuses.sort(compareStatusesForDropdown);
+ return [...statuses].sort(compareStatusesForDropdown);
}, [statuses]);📝 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.
| const sortedStatuses = useMemo(() => { | |
| return statuses.sort(compareStatusesForDropdown); | |
| }, [statuses]); | |
| const sortedStatuses = useMemo(() => { | |
| return [...statuses].sort(compareStatusesForDropdown); | |
| }, [statuses]); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/StatusProperty/StatusProperty.tsx
around lines 35 - 37, sortedStatuses is created by memoizing
statuses.sort(compareStatusesForDropdown) which mutates the original statuses
array; update the useMemo in StatusProperty (the sortedStatuses computation) to
first copy the array (e.g., via statuses.slice() or [...statuses]) and then call
sort(compareStatusesForDropdown) on the copy so the original statuses array is
not mutated.
| import { useState } from "react"; | ||
| import { HiCheck, HiChevronDown, HiOutlineClipboard } from "react-icons/hi2"; | ||
|
|
||
| const LANGUAGES = [ |
There was a problem hiding this comment.
Handle clipboard failures and avoid the magic number.
navigator.clipboard.writeText can reject; log with a prefixed message and move the timeout to a named constant.
🔧 Proposed fix
const LANGUAGES = [
{ value: "plaintext", label: "Plaintext" },
@@
{ value: "markdown", label: "Markdown" },
];
+
+const COPY_RESET_MS = 2000;
@@
const handleCopy = async () => {
- await navigator.clipboard.writeText(node.textContent);
- setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ try {
+ await navigator.clipboard.writeText(node.textContent);
+ setCopied(true);
+ setTimeout(() => setCopied(false), COPY_RESET_MS);
+ } catch (error) {
+ console.warn("[tasks/code-block-copy] Failed to copy", { error });
+ }
};Also applies to: 48-52
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx
at line 12, The copy-to-clipboard flow in CodeBlockView currently uses
navigator.clipboard.writeText without handling rejection and uses a magic
timeout; update the CodeBlockView component to (1) introduce a named constant
like CLIPBOARD_FEEDBACK_DURATION_MS for the timeout instead of the literal
number, (2) wrap navigator.clipboard.writeText(...) in a try/catch or promise
.catch and on error call console.error or processLogger.error with a prefixed
message (e.g., "CodeBlockView: clipboard write failed:") plus the error, and (3)
ensure success/failure feedback still uses CLIPBOARD_FEEDBACK_DURATION_MS; also
keep LANGUAGES as-is but ensure any related UI toggles rely on the new constant.
| function isLinearImageUrl(src: string): boolean { | ||
| try { | ||
| const url = new URL(src); | ||
| return url.host === LINEAR_IMAGE_HOST; | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Don’t swallow URL parse errors silently.
Log malformed URLs with context so failures are observable.
🔧 Proposed fix
function isLinearImageUrl(src: string): boolean {
try {
const url = new URL(src);
return url.host === LINEAR_IMAGE_HOST;
- } catch {
+ } catch (error) {
+ console.warn("[tasks/linear-image] Invalid image URL", { src, error });
return false;
}
}🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskMarkdownRenderer/components/LinearImage/LinearImage.tsx
around lines 8 - 14, The isLinearImageUrl function currently swallows URL parse
errors; update its catch block to log the malformed src and the caught error
(e.g., include src and error.message) before returning false so failures are
observable; modify the catch in isLinearImageUrl to call the app's logger (or
console as a fallback) with context including LINEAR_IMAGE_HOST and the input
src, then return false, leaving the rest of the URL parsing logic unchanged.
| const task: TaskWithStatus | null = useMemo(() => { | ||
| if (!taskData || taskData.length === 0) return null; | ||
| return taskData[0]; | ||
| }, [taskData]); | ||
|
|
||
| const handleBack = () => { | ||
| navigate({ to: "/tasks" }); | ||
| }; | ||
|
|
||
| const handleSaveDescription = (markdown: string) => { | ||
| if (!task) return; | ||
| collections.tasks.update(task.id, (draft) => { | ||
| draft.description = markdown; | ||
| }); | ||
| }; | ||
|
|
||
| if (!task) { | ||
| return ( | ||
| <div className="flex-1 flex items-center justify-center"> | ||
| <span className="text-muted-foreground">Task not found</span> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/page.tsx" | head -100Repository: superset-sh/superset
Length of output: 396
🏁 Script executed:
find "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks" -type f -name "*.tsx" -o -name "*.ts" | head -20Repository: superset-sh/superset
Length of output: 2729
🏁 Script executed:
find "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId" -maxdepth 1 -name "*.tsx" -o -name "*.ts"Repository: superset-sh/superset
Length of output: 137
🏁 Script executed:
ls -la "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/\$taskId/" 2>/dev/null || ls -la "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/" | grep -E "page|tsx"Repository: superset-sh/superset
Length of output: 347
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/\$taskId/page.tsx"Repository: superset-sh/superset
Length of output: 4846
🏁 Script executed:
rg "useLiveQuery" --type tsx --type ts -A 3 "apps/desktop/src/renderer" | head -60Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg "useLiveQuery" "apps/desktop/src/renderer" -A 3 | head -80Repository: superset-sh/superset
Length of output: 10085
🏁 Script executed:
rg "useLiveQuery.*isLoading" "apps/desktop/src/renderer" -B 2 | head -40Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx" | head -80Repository: superset-sh/superset
Length of output: 2982
🏁 Script executed:
rg "type.*useLiveQuery|interface.*useLiveQuery" "apps/desktop/src" -A 10 | head -40Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg "isLoading.*useLiveQuery|data:.*useLiveQuery" "apps/desktop/src/renderer" -B 1 -A 1 | head -60Repository: superset-sh/superset
Length of output: 6775
🏁 Script executed:
rg "@tanstack/react-db" "apps/desktop/src" --type json | grep -v node_modules | head -5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat "package.json" | grep -A 2 -B 2 "tanstack/react-db"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
find . -maxdepth 3 -name "package.json" -type f | head -5 | xargs grep -l "tanstack/react-db" 2>/dev/nullRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
grep "tanstack/react-db" ./apps/desktop/package.jsonRepository: superset-sh/superset
Length of output: 97
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/\$taskId/page.tsx" | tail -70Repository: superset-sh/superset
Length of output: 2272
Distinguish loading state from "not found" state.
The component currently conflates loading with not-found: when useLiveQuery is fetching data, taskData is falsy and triggers the "Task not found" message. Extract and check isLoading before rendering the not-found placeholder to prevent misleading flashes during initial load.
const { data: taskData, isLoading } = useLiveQuery(
(q) =>
q
.from({ tasks: collections.tasks })
.innerJoin({ status: collections.taskStatuses }, ({ tasks, status }) =>
eq(tasks.statusId, status.id),
)
.leftJoin({ assignee: collections.users }, ({ tasks, assignee }) =>
eq(tasks.assigneeId, assignee.id),
)
.select(({ tasks, status, assignee }) => ({
...tasks,
status,
assignee: assignee ?? null,
}))
.where(({ tasks }) => eq(tasks.id, taskId)),
[collections, taskId],
);
const task: TaskWithStatus | null = useMemo(() => {
if (!taskData || taskData.length === 0) return null;
return taskData[0];
}, [taskData]);
+if (isLoading) {
+ return (
+ <div className="flex-1 flex items-center justify-center">
+ <span className="text-muted-foreground">Loading task…</span>
+ </div>
+ );
+}
if (!task) {
return (
<div className="flex-1 flex items-center justify-center">
<span className="text-muted-foreground">Task not found</span>
</div>
);
}🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/page.tsx
around lines 50 - 72, The component currently treats a falsy taskData as "Task
not found"; instead, read the loading state from your data hook (e.g., the
isLoading/ready flag returned by useLiveQuery or whichever hook provides it) and
check that before showing the not-found UI. Update the logic where task is
derived (task, taskData) and the early return that renders "Task not found" so
it first returns a loading placeholder while isLoading is true, and only shows
the not-found message when isLoading is false and task is null; keep existing
handlers like handleSaveDescription and handleBack unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/EditableTitle/EditableTitle.tsx:
- Around line 26-35: When Escape is pressed the component schedules
setLocalValue(value) then immediately calls inputRef.current?.blur(), which can
let handleBlur read the stale (user-typed) localValue and call onSave; fix this
by adding a cancel ref flag (e.g., cancelRef) and updating both handlers: in
handleKeyDown when key === "Escape" set cancelRef.current = true, call
setLocalValue(value) and blur(), and in handleBlur check cancelRef.current first
— if true, clear the flag and return early (skip onSave); keep existing behavior
for Enter/normal blur. Update references to cancelRef in the functions
handleKeyDown and handleBlur and ensure cancelRef is created via useRef(false).
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/EditableTitle/EditableTitle.tsx (1)
37-48: Consider adding anaria-labelfor accessibility.The input lacks an accessible label. Screen reader users won't have context for what this input edits.
♿ Suggested improvement
<input ref={inputRef} type="text" value={localValue} onChange={(e) => setLocalValue(e.target.value)} onBlur={handleBlur} onKeyDown={handleKeyDown} + aria-label="Task title" className="w-full text-2xl font-semibold mb-6 p-0 bg-transparent border-none outline-none focus:outline-none placeholder:text-muted-foreground" placeholder="Task title..." />
| const handleKeyDown = (e: React.KeyboardEvent) => { | ||
| if (e.key === "Enter") { | ||
| e.preventDefault(); | ||
| inputRef.current?.blur(); | ||
| } | ||
| if (e.key === "Escape") { | ||
| setLocalValue(value); | ||
| inputRef.current?.blur(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Potential race condition: Escape may trigger save instead of cancel.
React state updates are asynchronous. When Escape is pressed, setLocalValue(value) schedules an update, but blur() fires immediately. The handleBlur callback may read the stale localValue (the user-typed value) before the state update commits, causing onSave to be called with the user's input instead of reverting.
🐛 Proposed fix using a cancel ref
export function EditableTitle({ value, onSave }: EditableTitleProps) {
const [localValue, setLocalValue] = useState(value);
const inputRef = useRef<HTMLInputElement>(null);
+ const isCancellingRef = useRef(false);
// Sync with external value changes
useEffect(() => {
setLocalValue(value);
}, [value]);
const handleBlur = () => {
+ if (isCancellingRef.current) {
+ isCancellingRef.current = false;
+ setLocalValue(value);
+ return;
+ }
const trimmed = localValue.trim();
if (trimmed && trimmed !== value) {
onSave(trimmed);
} else {
setLocalValue(value);
}
};
const handleKeyDown = (e: React.KeyboardEvent) => {
if (e.key === "Enter") {
e.preventDefault();
inputRef.current?.blur();
}
if (e.key === "Escape") {
+ isCancellingRef.current = true;
setLocalValue(value);
inputRef.current?.blur();
}
};📝 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.
| const handleKeyDown = (e: React.KeyboardEvent) => { | |
| if (e.key === "Enter") { | |
| e.preventDefault(); | |
| inputRef.current?.blur(); | |
| } | |
| if (e.key === "Escape") { | |
| setLocalValue(value); | |
| inputRef.current?.blur(); | |
| } | |
| }; | |
| export function EditableTitle({ value, onSave }: EditableTitleProps) { | |
| const [localValue, setLocalValue] = useState(value); | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const isCancellingRef = useRef(false); | |
| // Sync with external value changes | |
| useEffect(() => { | |
| setLocalValue(value); | |
| }, [value]); | |
| const handleBlur = () => { | |
| if (isCancellingRef.current) { | |
| isCancellingRef.current = false; | |
| setLocalValue(value); | |
| return; | |
| } | |
| const trimmed = localValue.trim(); | |
| if (trimmed && trimmed !== value) { | |
| onSave(trimmed); | |
| } else { | |
| setLocalValue(value); | |
| } | |
| }; | |
| const handleKeyDown = (e: React.KeyboardEvent) => { | |
| if (e.key === "Enter") { | |
| e.preventDefault(); | |
| inputRef.current?.blur(); | |
| } | |
| if (e.key === "Escape") { | |
| isCancellingRef.current = true; | |
| setLocalValue(value); | |
| inputRef.current?.blur(); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/EditableTitle/EditableTitle.tsx
around lines 26 - 35, When Escape is pressed the component schedules
setLocalValue(value) then immediately calls inputRef.current?.blur(), which can
let handleBlur read the stale (user-typed) localValue and call onSave; fix this
by adding a cancel ref flag (e.g., cancelRef) and updating both handlers: in
handleKeyDown when key === "Escape" set cancelRef.current = true, call
setLocalValue(value) and blur(), and in handleBlur check cancelRef.current first
— if true, clear the flag and return early (skip onSave); keep existing behavior
for Enter/normal blur. Update references to cancelRef in the functions
handleKeyDown and handleBlur and ensure cancelRef is created via useRef(false).
- Add BubbleMenu component with text selection toolbar - Support heading levels, bold, italic, underline, strike, link, code, quote - Add Underline extension to TipTap editor - Style task list checkboxes to match app design system - Use CSS custom properties for theme-aware checkbox colors
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskMarkdownRenderer/TaskMarkdownRenderer.tsx:
- Around line 126-229: The editor's content prop is only applied on mount via
useEditor so external updates aren't reflected; add a useEffect that watches
content and editor and calls editor.commands.setContent(content) when content
changes (guard by comparing to editor.getJSON() or checking
editor.isFocused()/unsaved state to avoid clobbering active edits). Locate the
editor instance created by useEditor and the onBlur handler in
TaskMarkdownRenderer, and implement the effect to sync incoming content prop
updates into the editor using editor.commands.setContent while avoiding
overwriting in-progress user edits.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/BubbleMenu/BubbleMenu.tsx (2)
56-65: Consider replacingwindow.promptwith a custom dialog.
window.prompt()provides a poor UX (blocks the thread, native browser styling, no input validation). Consider using a custom modal/popover for link input that can validate URLs and match your design system.Additionally, no URL validation is performed before setting the link. Invalid URLs could be set.
103-112: Consider addingaria-pressedfor better accessibility.Toggle buttons should indicate their pressed state to assistive technologies. This applies to all formatting buttons (Bold, Italic, Underline, Strikethrough, Link, Code, Blockquote).
♿ Example fix for Bold button
<button type="button" onClick={() => editor.chain().focus().toggleBold().run()} className={`h-7 w-7 flex items-center justify-center rounded hover:bg-accent transition-colors ${ editor.isActive("bold") ? "bg-accent" : "" }`} title="Bold" + aria-pressed={editor.isActive("bold")} >apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/TaskMarkdownRenderer.tsx (1)
37-52: Extract Linear image utility functions to shared module.The
isLinearImageUrlandgetLinearProxyUrlfunctions are duplicated in bothTaskMarkdownRenderer.tsxandLinearImage.tsxwith identical implementations. Extract these to a shared utility module within the TaskMarkdownRenderer feature folder (e.g.,utils/linearImage.ts) to maintain DRY principles and simplify future maintenance.
| const editor = useEditor({ | ||
| extensions: [ | ||
| Document, | ||
| Text, | ||
| Paragraph.configure({ | ||
| HTMLAttributes: { class: "mt-0 mb-3 leading-relaxed" }, | ||
| }), | ||
| StyledHeading.configure({ levels: [1, 2, 3, 4, 5, 6] }), | ||
| Bold.configure({ | ||
| HTMLAttributes: { class: "font-semibold" }, | ||
| }), | ||
| Italic.configure({ | ||
| HTMLAttributes: { class: "italic" }, | ||
| }), | ||
| Strike.configure({ | ||
| HTMLAttributes: { class: "line-through" }, | ||
| }), | ||
| Underline.configure({ | ||
| HTMLAttributes: { class: "underline" }, | ||
| }), | ||
| Code.configure({ | ||
| HTMLAttributes: { | ||
| class: "font-mono text-sm px-1 py-0.5 rounded bg-muted", | ||
| }, | ||
| }), | ||
| CodeBlockLowlight.extend({ | ||
| addNodeView() { | ||
| return ReactNodeViewRenderer(CodeBlockView); | ||
| }, | ||
| }).configure({ | ||
| lowlight, | ||
| HTMLAttributes: { | ||
| class: | ||
| "my-3 p-3 rounded-md bg-muted overflow-x-auto font-mono text-sm", | ||
| }, | ||
| }), | ||
| BulletList.configure({ | ||
| HTMLAttributes: { | ||
| class: "task-markdown-list mt-0 pl-6", | ||
| }, | ||
| }), | ||
| OrderedList.configure({ | ||
| HTMLAttributes: { class: "mt-0 mb-3 pl-6 list-decimal" }, | ||
| }), | ||
| ListItem.configure({ | ||
| HTMLAttributes: {}, | ||
| }), | ||
| TaskList.configure({ | ||
| HTMLAttributes: { class: "mt-0 mb-3 pl-0 list-none" }, | ||
| }), | ||
| TaskItem.configure({ | ||
| HTMLAttributes: { class: "flex items-start gap-2 mb-1" }, | ||
| nested: true, | ||
| }), | ||
| Blockquote.configure({ | ||
| HTMLAttributes: { | ||
| class: "my-3 pl-4 border-l-2 border-border text-muted-foreground", | ||
| }, | ||
| }), | ||
| HorizontalRule.configure({ | ||
| HTMLAttributes: { class: "my-6 border-none border-t border-border" }, | ||
| }), | ||
| HardBreak, | ||
| Link.configure({ | ||
| openOnClick: false, | ||
| HTMLAttributes: { class: "text-primary underline" }, | ||
| }), | ||
| LinearImage.configure({ | ||
| HTMLAttributes: { class: "max-w-full h-auto rounded-md my-3" }, | ||
| }), | ||
| Placeholder.configure({ | ||
| placeholder: ({ node }) => { | ||
| if (node.type.name === "paragraph") { | ||
| return "Add description..."; | ||
| } | ||
| return ""; | ||
| }, | ||
| showOnlyCurrent: false, | ||
| emptyNodeClass: | ||
| "first:before:text-muted-foreground first:before:float-left first:before:h-0 first:before:pointer-events-none first:before:content-[attr(data-placeholder)]", | ||
| }), | ||
| Markdown.configure({ | ||
| html: true, | ||
| transformPastedText: true, | ||
| transformCopiedText: true, | ||
| }), | ||
| SlashCommand, | ||
| KeyboardHandler, | ||
| ], | ||
| content, | ||
| editorProps: { | ||
| attributes: { | ||
| class: "focus:outline-none min-h-[100px]", | ||
| }, | ||
| }, | ||
| onBlur: ({ editor }) => { | ||
| const storage = editor.storage as unknown as Record< | ||
| string, | ||
| { getMarkdown?: () => string } | ||
| >; | ||
| const markdown = storage.markdown?.getMarkdown?.() ?? ""; | ||
| onSave(markdown); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "TaskMarkdownRenderer" --type=ts --type=tsx -B 2 -A 10Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg "TaskMarkdownRenderer" --type ts -B 2 -A 10Repository: superset-sh/superset
Length of output: 6400
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/page.tsx" | head -80Repository: superset-sh/superset
Length of output: 396
🏁 Script executed:
find apps/desktop/src/renderer/routes -name "page.tsx" -path "*tasks*" | head -20Repository: superset-sh/superset
Length of output: 219
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/\$taskId/page.tsx" | head -150Repository: superset-sh/superset
Length of output: 5113
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/\$taskId/components/TaskMarkdownRenderer/TaskMarkdownRenderer.tsx"Repository: superset-sh/superset
Length of output: 8661
Add a useEffect to sync content prop changes to the editor.
useEditor only uses the content prop for initial setup. Since this component receives content from a reactive useLiveQuery source, when content changes after mount (e.g., from external updates via useLiveQuery or after the user saves), the editor won't reflect those changes. Add a useEffect that calls editor?.commands.setContent(content) when the content prop changes:
useEffect(() => {
if (editor && content !== editor.getJSON()) {
editor.commands.setContent(content);
}
}, [content, editor]);Consider whether to check if the editor is focused/has unsaved changes before updating, to avoid overwriting user edits in progress.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/TaskMarkdownRenderer/TaskMarkdownRenderer.tsx
around lines 126 - 229, The editor's content prop is only applied on mount via
useEditor so external updates aren't reflected; add a useEffect that watches
content and editor and calls editor.commands.setContent(content) when content
changes (guard by comparing to editor.getJSON() or checking
editor.isFocused()/unsaved state to avoid clobbering active edits). Locate the
editor instance created by useEditor and the onBlur handler in
TaskMarkdownRenderer, and implement the effect to sync incoming content prop
updates into the editor using editor.commands.setContent while avoiding
overwriting in-progress user edits.
saddlepaddle
changed the title
- Remove BubbleMenu component (moved to SUPER-258 for future implementation) - Add @tiptap/extension-history for undo/redo (Cmd+Z / Cmd+Shift+Z)
1 participant
Summary
/tasks/$taskIdwith Linear-style UI (properties sidebar)uploads.linear.apprequires authentication)Test plan
bun run typecheckandbun run lintSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.