Add: in-row-editor navigation #2538
Conversation
Console (appwrite/console)Project ID: Tip Appwrite has crossed the 50K GitHub stars milestone with hundreds of active contributors |
WalkthroughThis PR introduces row navigation controls to a database table editor. The changes add up/down chevron buttons in the sidesheet header that allow users to navigate between rows in the current dataset, with controls disabled at boundaries and hidden on tablet viewports. The implementation extends the store with row context (array of rows and current index), manages focus behavior through a new autoFocus flag, adds spatial type comparison support, and coordinates these across multiple layout and editor components to enable seamless row traversal. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(3 hunks)
🧰 Additional context used
🧠 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]/table-[table]/rows/edit.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.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]/table-[table]/rows/edit.svelte
📚 Learning: 2025-09-04T03:59:03.485Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2290
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte:952-961
Timestamp: 2025-09-04T03:59:03.485Z
Learning: In the Appwrite console spreadsheet component, for relationship columns, the columnId corresponds to the tableId of the related table. When calling openSideSheetForRelationsToMany(tableId, rows), the columnId can be passed directly as the tableId parameter for relationship columns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.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]/table-[table]/rows/edit.svelte
...e)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (2)
551-561: Guard against null row before accessing properties.The past review comment's concern remains unaddressed. When
rowis passed as null or undefined (e.g., when the focused row isn't found in line 809's lookup), dereferencingrow.$idat line 552 will throw a runtime error before the sheet opens.Apply this diff to restore the null guard:
if (action === 'update') { databaseRowSheetOptions.update((opts) => { + if (!row) { + return opts; + } const rowIndex = rowIndexMap.get(row.$id) ?? -1; return {
807-814: Verify focused row exists before opening editor.When
Cmd+Enteris pressed, the code searches$paginatedRows.itemsfor the focused row. If the row withfocusedRowIddoesn't exist in the current page cache,focusedRowwill beundefined. Passing this toonSelectSheetOptionwill trigger the null-access issue at line 552.Consider adding a guard after the lookup:
expandKbdShortcut="Cmd+Enter" on:expandKbdShortcut={({ detail }) => { const focusedRowId = detail.rowId; const focusedRow = $paginatedRows.items.find((row) => row.$id === focusedRowId); + if (!focusedRow) return; + previouslyFocusedElement = document.activeElement; $databaseRowSheetOptions.autoFocus = false; onSelectSheetOption('update', null, 'row', focusedRow); }}>
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
90-114: Manual focus management adds complexity.The
autoFocusActionhelper manually manages focus andfocus-visibleclass toggling. While this works, it introduces complexity and potential maintenance burden. The need for manual class management suggests the underlying component library may not handle this use case natively.If the Pink Svelte library supports focus management via props or if a simpler approach exists, consider refactoring to reduce custom focus logic. Otherwise, ensure this pattern is documented for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/sidesheet.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(6 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/sidesheet.svelte
🧰 Additional context used
🧠 Learnings (7)
📚 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]/spreadsheet.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.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]/table-[table]/+layout.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.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]/table-[table]/+layout.svelte
📚 Learning: 2025-09-26T06:48:57.938Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:629-631
Timestamp: 2025-09-26T06:48:57.938Z
Learning: In the Appwrite console codebase using appwrite.io/pink-svelte, the Icon component automatically handles CSS variable names passed to its color prop by internally wrapping them with var(). Therefore, passing '--some-css-variable' as a string to the Icon color prop works correctly without needing to manually wrap it with var().
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.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]/table-[table]/rows/edit.svelte
📚 Learning: 2025-09-04T03:59:03.485Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2290
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte:952-961
Timestamp: 2025-09-04T03:59:03.485Z
Learning: In the Appwrite console spreadsheet component, for relationship columns, the columnId corresponds to the tableId of the related table. When calling openSideSheetForRelationsToMany(tableId, rows), the columnId can be passed directly as the tableId parameter for relationship columns.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.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]/table-[table]/rows/edit.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). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (10)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (2)
115-116: LGTM! Efficient row index lookup.The reactive
rowIndexMapprovides O(1) lookups for row indices, which is an efficient solution for the navigation feature.
812-813: Consistent autoFocus flag management.The
autoFocusflag is being set consistently across different user interaction points (keyboard shortcuts, expand button clicks, and inline editor actions) to control focus behavior appropriately.Also applies to: 939-939, 991-991, 1120-1120
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/edit.svelte (4)
103-105: Spatial type comparison handles complex objects correctly.Using
deepEqualfor spatial type comparisons is the right approach, as these types (point, linestring, polygon) are complex objects that require deep equality checks rather than reference equality.
87-89: AutoFocus flag correctly gates input focus.The conditional focus behavior respects the
autoFocusflag, preventing unwanted focus when navigating between rows via keyboard shortcuts.
28-29: AutoFocus prop aligns with store changes.The new
autoFocusprop with a default oftrueintegrates well with the broader store changes that enable row navigation context.Also applies to: 33-33
21-21: All concerns verified—deep-equal is properly installed and supports the default import syntax used.The deep-equal package (v^2.2.3) is confirmed in dependencies and exports a default export, supporting the import statement
import deepEqual from 'deep-equal';as used in the code. No issues found.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (4)
486-535: Navigation controls with proper boundary checks.The chevron up/down buttons correctly:
- Calculate current position using rowIndex
- Disable at boundaries (first/last row)
- Update the store with the adjacent row
- Hide on tablet viewports
The focus logic (
shouldFocusPrev/shouldFocusNext) correctly determines which button to focus based on position: down button when at the first row, up button otherwise.
492-492: Tablet viewport guard preserves mobile experience.Hiding the navigation controls on tablet viewports is a sensible UX decision given the likely space constraints.
541-542: AutoFocus binding completes the navigation flow.Passing
$databaseRowSheetOptions.autoFocusto theEditRowcomponent properly coordinates focus behavior between the navigation controls and the form inputs.
613-617: Custom focus outline styling.The custom
focus-visibleoutline ensures accessibility while maintaining visual consistency with the design system.
What does this PR do?
(Provide a description of what this PR does.)
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
Chores