feat: right click actions across the console #2787
Conversation
WalkthroughAdds nine new analytics Click enum members and a trailing-comma adjustment. Introduces RowContextMenu and TableContextMenu Svelte components with onSelect callbacks and nested Copy submenus. Re-enables the 'copy-snippet' RowCellAction, adds a rowCopySnippetSheet store, and introduces a CopySnippetModal component. Updates spreadsheet.svelte to use RowContextMenu, expands onSelectSheetOption to accept a source parameter, and emits trackEvent calls with source metadata. Adds showLabel prop to InputSelect and implements CSV import, pin/unpin, and table-action handling in subNavigation and related files. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte:
- Around line 89-96: The copy submenu incorrectly instantiates an
ActionMenu.Root for each copyMenuItems entry; wrap a single ActionMenu.Root
(instead of one per item) around the {`#each` copyMenuItems as copyItem} loop so
all ActionMenu.Item.Button elements live under the same ActionMenu.Root; keep
the existing use:melt={$copySubTrigger} and per-item on:m-click={() =>
onSelect(copyItem.action)} handlers inside each ActionMenu.Item.Button and
ensure you remove the inner ActionMenu.Roots so only the outer ActionMenu.Root
encloses the loop.
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte (1)
62-62: UseIconClipboardCopyfor the Copy submenu.
IconDuplicateis already used for "Duplicate row" (line 57). Using the same icon for "Copy" may confuse users. SinceIconClipboardCopyis already imported, consider using it here for better semantic clarity.Suggested fix
- { label: 'Copy', icon: IconDuplicate, subMenu: 'copy' }, + { label: 'Copy', icon: IconClipboardCopy, subMenu: 'copy' },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte (4)
1-17: LGTM on imports.Correct use of
$libalias for analytics and appropriate external package imports.
74-131: Template implementation looks correct.The melt-ui bindings for trigger, menu, item, and submenu are properly applied. The conditional rendering logic correctly handles dividers, submenu triggers, and regular action items.
133-154: Styles look good.Appropriate use of CSS custom properties for border-radius and shadows. The z-index of 50 is reasonable for a context menu overlay.
19-19: The codebase consistently usesexport letfor component props throughout the databases directory and beyond. Suggesting migration to$props()rune contradicts the established pattern in this project, which has not yet adopted that syntax despite the coding guidelines mentioning Svelte 5.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <div use:melt={$copySubTrigger} class="sub-trigger"> | ||
| <ActionMenu.Root> | ||
| <ActionMenu.Item.Button | ||
| leadingIcon={menuItem.icon} | ||
| trailingIcon={IconChevronRight}> | ||
| {menuItem.label} | ||
| </ActionMenu.Item.Button> | ||
| </ActionMenu.Root> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how ActionMenu.Root is typically used in the codebase
rg -n -B2 -A10 'ActionMenu\.Root' --type=svelte src/routes/ | head -100Repository: appwrite/console
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for ActionMenu.Root usage in .svelte files
rg -n -B3 -A8 'ActionMenu\.Root' src/routes/ --glob='*.svelte' | head -150Repository: appwrite/console
Length of output: 18237
🏁 Script executed:
#!/bin/bash
# Get the full context of the rowContextMenu.svelte file
cat -n src/routes/\(console\)/project-\[region\]-\[project\]/databases/database-\[database\]/table-\[table\]/rowContextMenu.svelte | sed -n '74,154p'Repository: appwrite/console
Length of output: 3576
Simplify ActionMenu.Root wrapping in copy submenu items (lines 120-124).
Each item in the copyMenuItems loop has its own ActionMenu.Root, which is atypical. The standard pattern across the codebase is one ActionMenu.Root wrapping multiple items. Move the ActionMenu.Root outside the loop:
Current structure (incorrect)
{`#each` copyMenuItems as copyItem}
<div use:melt={$item} on:m-click={() => onSelect(copyItem.action)}>
<ActionMenu.Root>
<ActionMenu.Item.Button leadingIcon={copyItem.icon}>
{copyItem.label}
</ActionMenu.Item.Button>
</ActionMenu.Root>
</div>
{/each}
Recommended: One ActionMenu.Root should wrap all items in the loop, consistent with how the main menu items are structured (lines 81-111).
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rowContextMenu.svelte
around lines 89 - 96, The copy submenu incorrectly instantiates an
ActionMenu.Root for each copyMenuItems entry; wrap a single ActionMenu.Root
(instead of one per item) around the {`#each` copyMenuItems as copyItem} loop so
all ActionMenu.Item.Button elements live under the same ActionMenu.Root; keep
the existing use:melt={$copySubTrigger} and per-item on:m-click={() =>
onSelect(copyItem.action)} handlers inside each ActionMenu.Item.Button and
ensure you remove the inner ActionMenu.Roots so only the outer ActionMenu.Root
encloses the loop.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte:
- Around line 135-140: The catch block assumes the caught value e has a message
property; instead normalize and defensively handle non-Error throwables: wrap or
convert e before calling trackError and addNotification by ensuring trackError
receives an Error (e instanceof Error ? e : new Error(String(e))) and
addNotification uses a safe message like (e && typeof e === 'object' &&
'message' in e ? e.message : String(e)); update the catch in the function
handling CSV import so it references trackError, Submit.DatabaseImportCsv, and
addNotification with the normalized values.
- Around line 155-170: The clipboard write operations in the switch cases for
'copy-url' and 'copy-json' use navigator.clipboard.writeText() without error
handling; wrap each await navigator.clipboard.writeText(...) call in try/catch
inside the same cases (the 'copy-url' and 'copy-json' branches) and on failure
call addNotification with type: 'error' and a helpful message (include the
error.message) so failures are reported; retain the success addNotification on
success.
- Around line 188-189: The 'delete' branch in subNavigation.svelte is empty and
needs to actually trigger table deletion; implement it by either navigating to
the table settings page (where the existing delete modal/component lives) or by
invoking the same deletion flow used in
table-[table]/settings/deleteTable.svelte. Locate the switch's 'case "delete"'
in subNavigation.svelte, import and use the router/navigation helper (e.g.,
goto/generatePath) to route to the table settings URL for the selected table OR
open a confirmation modal that calls the same delete handler used by
deleteTable.svelte (reuse its deletion function or extract its API call into a
shared helper) and handle success/failure UI updates and refresh of the table
list.
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.svelte (3)
41-46: Remove or implement the commented tracking code.The commented
trackEventcall suggests incomplete implementation. Either implement the tracking or remove the dead comment to keep the code clean.♻️ Suggested change
onOpenChange: ({ next }) => { - if (next) { - // trackEvent(Click.TableContextMenuOpen); // Define click event if needed - } return next; }
121-136: RedundantActionMenu.Rootwrappers in submenu items.Each
copyItemis wrapped in its ownActionMenu.Root, but these items are already within the submenu context. This pattern differs from the main menu (lines 88-118) where a singleActionMenu.Rootwraps all items. Consider consolidating for consistency.♻️ Suggested refactor
<div class="menu submenu" use:melt={$copySubMenu}> <Card.Base padding="none"> <div class="action-menu-root"> + <ActionMenu.Root> {`#each` copyMenuItems as copyItem} <div use:melt={$item} on:m-click={() => onSelect(copyItem.action)}> - <ActionMenu.Root> - <ActionMenu.Item.Button leadingIcon={copyItem.icon}> - {copyItem.label} - </ActionMenu.Item.Button> - </ActionMenu.Root> + <ActionMenu.Item.Button leadingIcon={copyItem.icon}> + {copyItem.label} + </ActionMenu.Item.Button> </div> {/each} + </ActionMenu.Root> </div> </Card.Base> </div>
59-72: Consider adding an explicit discriminated union type formenuItemsto improve type safety.The
menuItemsarray mixes three different object shapes (action items withactionproperty, divider items withdivider: true, and submenu items withsubMenu), which causes TypeScript to require a type assertion on line 109:menuItem.action as TableAction. A discriminated union type would eliminate the need for this cast and make the structure clearer.Note that
'download-csv'is included in theTableActiontype but is never used anywhere in the codebase—it may be dead code or intended for future use.src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (1)
368-373: Consider conditionally renderingCopySnippetModal.Unlike
FilePickerwhich is conditionally rendered with{#ifshowImportCSV},CopySnippetModalis always in the DOM. For consistency and potential performance benefits, consider conditional rendering.♻️ Suggested change
+{`#if` showCopySnippetModal} <CopySnippetModal bind:show={showCopySnippetModal} row={null} {databaseId} collectionId={selectedTableForAction?.$id ?? ''} /> +{/if}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
🧠 Learnings (5)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-10-13T05:16:07.656Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-10-05T09:41:40.439Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2398
File: src/routes/(console)/verify-email/+page.svelte:48-51
Timestamp: 2025-10-05T09:41:40.439Z
Learning: In SvelteKit 5, `page` imported from `$app/state` is a reactive state object (using runes), not a store. It should be accessed as `page.data` without the `$` prefix, unlike the store-based `$page` from `$app/stores` in earlier versions.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-09-25T04:33:19.632Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/createTable.svelte:28-33
Timestamp: 2025-09-25T04:33:19.632Z
Learning: In the Appwrite console createTable component, manual submit state management (like setting creatingTable = true) is not needed because: 1) The Modal component handles submit state internally via submissionLoader, preventing double submissions, and 2) After successful table creation, the entire view is destroyed and replaced with the populated table view, ending the component lifecycle.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
🔇 Additional comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/tableContextMenu.svelte (2)
1-11: LGTM!The
TableActiontype union is well-defined and exported from the module context for reuse in other components.
140-161: LGTM!The styling provides proper z-index layering, dimensions, and visual polish for the context menu and submenu.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (3)
33-40: LGTM!New imports are properly organized and use the
$libalias as per coding guidelines.
57-74: LGTM!The pinned tables logic correctly:
- Derives a unique key per database
- Safely parses the comma-separated preference string with optional chaining and filters empty values
- Sorts tables with pinned items first, then alphabetically
232-252: LGTM!The
TableContextMenuintegration correctly wraps each table item, passes the appropriateisPinnedstate, and delegates action handling tohandleTableAction.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| } catch (e) { | ||
| trackError(e, Submit.DatabaseImportCsv); | ||
| addNotification({ | ||
| type: 'error', | ||
| message: e.message | ||
| }); |
There was a problem hiding this comment.
Add defensive error handling for the catch block.
The error e is assumed to have a message property, but if the caught value isn't an Error instance, this could fail or show undefined.
🐛 Suggested fix
} catch (e) {
trackError(e, Submit.DatabaseImportCsv);
addNotification({
type: 'error',
- message: e.message
+ message: e instanceof Error ? e.message : 'Failed to import CSV'
});
} finally {📝 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.
| } catch (e) { | |
| trackError(e, Submit.DatabaseImportCsv); | |
| addNotification({ | |
| type: 'error', | |
| message: e.message | |
| }); | |
| } catch (e) { | |
| trackError(e, Submit.DatabaseImportCsv); | |
| addNotification({ | |
| type: 'error', | |
| message: e instanceof Error ? e.message : 'Failed to import CSV' | |
| }); |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
around lines 135 - 140, The catch block assumes the caught value e has a message
property; instead normalize and defensively handle non-Error throwables: wrap or
convert e before calling trackError and addNotification by ensuring trackError
receives an Error (e instanceof Error ? e : new Error(String(e))) and
addNotification uses a safe message like (e && typeof e === 'object' &&
'message' in e ? e.message : String(e)); update the catch in the function
handling CSV import so it references trackError, Submit.DatabaseImportCsv, and
addNotification with the normalized values.
| case 'copy-url': | ||
| await navigator.clipboard.writeText( | ||
| `${window.location.origin}${base}/project-${region}-${project}/databases/database-${databaseId}/table-${table.$id}` | ||
| ); | ||
| addNotification({ | ||
| type: 'success', | ||
| message: 'URL copied to clipboard' | ||
| }); | ||
| break; | ||
| case 'copy-json': | ||
| await navigator.clipboard.writeText(JSON.stringify(table, null, 2)); | ||
| addNotification({ | ||
| type: 'success', | ||
| message: 'JSON copied to clipboard' | ||
| }); | ||
| break; |
There was a problem hiding this comment.
Add error handling for clipboard operations.
navigator.clipboard.writeText() can fail (e.g., permissions denied, insecure context). Consider wrapping in try/catch to handle failures gracefully.
🐛 Suggested fix for copy-url case
case 'copy-url':
+ try {
await navigator.clipboard.writeText(
`${window.location.origin}${base}/project-${region}-${project}/databases/database-${databaseId}/table-${table.$id}`
);
addNotification({
type: 'success',
message: 'URL copied to clipboard'
});
+ } catch {
+ addNotification({
+ type: 'error',
+ message: 'Failed to copy URL to clipboard'
+ });
+ }
break;🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
around lines 155 - 170, The clipboard write operations in the switch cases for
'copy-url' and 'copy-json' use navigator.clipboard.writeText() without error
handling; wrap each await navigator.clipboard.writeText(...) call in try/catch
inside the same cases (the 'copy-url' and 'copy-json' branches) and on failure
call addNotification with type: 'error' and a helpful message (include the
error.message) so failures are reported; retain the success addNotification on
success.
...utes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte:
- Around line 173-176: The catch block that sets deleteError = e.message (and
mirrors the same issue in onSelectFile) assumes the thrown value is an Error;
change it to defensively extract a message: compute a safeMessage like (e &&
typeof e === 'object' && 'message' in e) ? String(e.message) : String(e) and
assign deleteError = safeMessage; continue to call trackError(e,
Submit.TableDelete) but consider passing a normalized Error or the safeMessage
if trackError expects an Error instance. Update the catch in the handler that
uses deleteError and the onSelectFile catch to use this same defensive
extraction.
♻️ Duplicate comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (2)
136-141: Add defensive error handling for the catch block.The error
eis assumed to have amessageproperty, but if the caught value isn't anErrorinstance, this could fail or showundefined.
188-203: Add error handling for clipboard operations.
navigator.clipboard.writeText()can fail (e.g., permissions denied, insecure context). Bothcopy-urlandcopy-jsoncases need try/catch.Based on learnings, the codebase has a
copyhelper utility insrc/lib/helpers/copy.tsthat handles errors internally and returns a boolean success indicator. Consider using it instead of rawnavigator.clipboard.writeText().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
🧠 Learnings (8)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-10-21T06:07:53.455Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2477
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:27-38
Timestamp: 2025-10-21T06:07:53.455Z
Learning: In the Appwrite Console codebase, the `copy` helper utility (src/lib/helpers/copy.ts) already handles errors internally with try-catch blocks and returns a boolean success indicator. Additional error handling with try-catch is not needed when calling this utility.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Cursor integration URL format `https://cursor.com/link/prompt` with the `text` query parameter is correct and functional.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-09-25T04:33:19.632Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/createTable.svelte:28-33
Timestamp: 2025-09-25T04:33:19.632Z
Learning: In the Appwrite console createTable component, manual submit state management (like setting creatingTable = true) is not needed because: 1) The Modal component handles submit state internally via submissionLoader, preventing double submissions, and 2) After successful table creation, the entire view is destroyed and replaced with the populated table view, ending the component lifecycle.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
📚 Learning: 2025-10-13T05:16:07.656Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte:54-58
Timestamp: 2025-10-13T05:16:07.656Z
Learning: In SvelteKit apps, shared layout components (like headers) that use `$derived(page.data.*)` should use optional chaining when accessing properties that may not be present on all routes. During page transitions, reactive statements can briefly evaluate with different page.data structures, so optional chaining prevents runtime errors when navigating between routes with different data shapes (e.g., between `/databases` and `/databases/database-[database]`).
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
🔇 Additional comments (6)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte (6)
33-41: LGTM!Imports are well-organized and correctly use the
$libalias as per coding guidelines.
58-75: LGTM!The pinned tables sorting logic is well-implemented. Pinned tables are correctly prioritized at the top while maintaining alphabetical order within each group.
221-223: LGTM!The delete action now properly triggers the confirmation modal, addressing the previously flagged empty case.
266-286: LGTM!Clean integration of
TableContextMenuwith proper callback wiring and pinned state tracking.
422-431: LGTM!The delete confirmation modal is properly configured with appropriate bindings and messaging.
402-406: TheCopySnippetModalusage withrow={null}is intentional and correct. The component is designed to handle both table-level and row-level contexts. Whenrow={null}, the component generates template snippets with<DOCUMENT_ID>as a placeholder, which is appropriate for table-level usage. This matches the pattern used elsewhere in the codebase (e.g.,spreadsheet.sveltepasses actual row objects for row-specific snippets).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| } catch (e) { | ||
| deleteError = e.message; | ||
| trackError(e, Submit.TableDelete); | ||
| } |
There was a problem hiding this comment.
Add defensive error handling for the catch block.
Same issue as in onSelectFile: the error e is assumed to have a message property. If the caught value isn't an Error instance, this will fail or show undefined.
🐛 Suggested fix
} catch (e) {
- deleteError = e.message;
+ deleteError = e instanceof Error ? e.message : 'Failed to delete table';
trackError(e, Submit.TableDelete);
}📝 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.
| } catch (e) { | |
| deleteError = e.message; | |
| trackError(e, Submit.TableDelete); | |
| } | |
| } catch (e) { | |
| deleteError = e instanceof Error ? e.message : 'Failed to delete table'; | |
| trackError(e, Submit.TableDelete); | |
| } |
🤖 Prompt for AI Agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/subNavigation.svelte
around lines 173 - 176, The catch block that sets deleteError = e.message (and
mirrors the same issue in onSelectFile) assumes the thrown value is an Error;
change it to defensively extract a message: compute a safeMessage like (e &&
typeof e === 'object' && 'message' in e) ? String(e.message) : String(e) and
assign deleteError = safeMessage; continue to call trackError(e,
Submit.TableDelete) but consider passing a normalized Error or the safeMessage
if trackError expects an Error instance. Update the catch in the handler that
uses deleteError and the onSelectFile catch to use this same defensive
extraction.
What does this PR do?
spreadsheets
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.