Revamp designs for Suggestions UI #2486
Conversation
update: allow passing a subtitle snippet. update: allow setting icons to secondary and primary actions. remove: confirm on indexes. remove: empty column on indexes when on a small viewport.
|
This PR was not deployed automatically as @ItzNotABug does not have access to the Railway project. In order to get automatic PR deploys, please add @ItzNotABug to your workspace on Railway. |
ConsoleProject ID: Sites (2)
Tip Appwrite has a Discord community with over 16 000 members. |
WalkthroughAdds an AI-driven column-suggestions feature and UI: new ColumnsSuggestions modal and related components (columns.svelte, input.svelte, options.svelte, empty.svelte, icon/aiForButton.svelte, EmptySheetCards, layout/emptySheetCards.svelte). Wires a showColumnsSuggestionsModal store and a force flag into tableColumnSuggestions. Reworks EmptySheet layout/overlay logic and exposes Snippet-based subtitle/actions. Introduces a global disabled prop propagated across many column editor and form components. Adds class/export tweaks to Card and a hoverEffect prop on Spreadsheet rows. Updates two @appwrite.io/pink-* dependencies in package.json. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/relationship.svelte (1)
188-207: Asymmetry confirmed: Primary key should also be readonly during editing.The verification reveals the asymmetry is real but indicates a design inconsistency:
- twoWayKey (line 207):
readonly={editing}is correct because the API call doesn't include anewTwoWayKeyparameter—it cannot be modified at all.- Primary key (line 195): The API call sends
newKey: data.key !== originalKey ? data.key : undefined(line 53), allowing the key to be renamed during updates.However, the helper text at lines 205-206 states: "Once created, column key cannot be adjusted to maintain data integrity." This applies to both keys conceptually, but only the twoWayKey is prevented from editing.
Add
readonly={editing}to the primary key input (line 195) to prevent modification after relationship creation, making the behavior consistent with the helper message and the twoWayKey logic.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (1)
1110-1136: Duplicate id="key" in repeated inputs; generate unique IDs per column.Repeated static IDs harm a11y and label association.
- <InputText - id="key" + <InputText + id={`key-${column.id}`} autofocus @@ - <InputText - id="key" + <InputText + id={`key-${id}`} label="Key"Also applies to: 1153-1160
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (2)
89-91: Null-safety for hasColumns to avoid runtime on undefined columnsDirect
.lengthon possibly undefined can throw. Mirror yourhasValidColumnsoptional chaining.- $: hasColumns = !!$table.columns.length; + $: hasColumns = !!$table?.columns?.length;
73-79: Inverted filter for system columns in Filters optionsYou exclude
$idwhen selected but include system columns only when selected. Likely both should exclude when selected.- ].filter((col) => !!selected.includes(col.id)); + ].filter((col) => !selected.includes(col.id));src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte (1)
95-104: Overlay offsets not updated on scroll; add listenersResizeObserver won’t catch scroll. Update offsets/heights on scroll/viewport resize to keep the overlay aligned.
onMount(async () => { if (spreadsheetContainer) { resizeObserver = new ResizeObserver(debouncedUpdateOverlayHeight); resizeObserver.observe(spreadsheetContainer); overlayOffsetHandler = new ResizeObserver(updateOverlayLeftOffset); overlayOffsetHandler.observe(spreadsheetContainer); } + + const => { + updateOverlayLeftOffset(); + debouncedUpdateOverlayHeight(); + }; + window.addEventListener('scroll', onScrollOrResize, { passive: true }); + window.addEventListener('resize', onScrollOrResize, { passive: true }); }); onDestroy(() => { if (resizeObserver) { resizeObserver.disconnect(); } if (overlayOffsetHandler) { overlayOffsetHandler.disconnect(); } + window.removeEventListener('scroll', onScrollOrResize); + window.removeEventListener('resize', onScrollOrResize); });Also applies to: 105-113
🧹 Nitpick comments (15)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/url.svelte (1)
59-69: Consider simplifying the reactive pattern.The
createConservativehelper adds indirection by creating stores that mirrordata.requiredanddata.array, syncing them vialisten(data), and then using those stores in a reactive statement. Since the UI binds directly todata.requiredanddata.array(lines 84, 92), this could be simplified to:-const { - stores: { required, array }, - listen -} = createConservative<Partial<Models.ColumnUrl>>({ - required: false, - array: false, - ...data -}); -$: listen(data); - -$: handleDefaultState($required || $array); +$: handleDefaultState(data.required || data.array);If
createConservativeprovides specific optimizations (e.g., preventing unnecessary re-renders), this pattern is acceptable, but the added complexity warrants verification.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/datetime.svelte (1)
63-74: Consider using stores consistently for reactivity.The
createConservativehelper creates reactive stores forrequiredandarray, which are used in thehandleDefaultStateeffect (line 74). However, the disabled expressions (lines 81, 89, 97) accessdata.requiredanddata.arraydirectly. This creates two sources of truth that might drift out of sync.If intentional for performance or reactivity reasons, this is fine. Otherwise, consider using either the stores consistently throughout or direct data access everywhere.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte (1)
235-249: Consider using CSS variables instead of hardcoded values.The custom styling uses hardcoded spacing values. For consistency with the design system, consider using existing CSS variable names if they're semantically appropriate.
.custom-inline-alert { & :global(article) { - padding: var(--space-4, 8px); + padding: var(--space-xs); }This assumes
--space-xsmaps to 8px in your design system. Ifvar(--space-4, 8px)is the correct pattern, then this change isn't needed.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/icon/aiForButton.svelte (1)
10-21: Consider using CSS variables for the icon color.The fill color is hardcoded to
#97979B. For better theming support and consistency with the design system, consider using a CSS variable.<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20" fill="none"> <path fill-rule="evenodd" clip-rule="evenodd" d="M5.00049 2C5.55277 2 6.00049 2.44772 6.00049 3V4H7.00049C7.55277 4 8.00049 4.44772 8.00049 5C8.00049 5.55228 7.55277 6 7.00049 6H6.00049V7C6.00049 7.55228 5.55277 8 5.00049 8C4.4482 8 4.00049 7.55228 4.00049 7V6H3.00049C2.4482 6 2.00049 5.55228 2.00049 5C2.00049 4.44772 2.4482 4 3.00049 4H4.00049V3C4.00049 2.44772 4.4482 2 5.00049 2ZM5.00049 12C5.55277 12 6.00049 12.4477 6.00049 13V14H7.00049C7.55277 14 8.00049 14.4477 8.00049 15C8.00049 15.5523 7.55277 16 7.00049 16H6.00049V17C6.00049 17.5523 5.55277 18 5.00049 18C4.4482 18 4.00049 17.5523 4.00049 17V16H3.00049C2.4482 16 2.00049 15.5523 2.00049 15C2.00049 14.4477 2.4482 14 3.00049 14H4.00049V13C4.00049 12.4477 4.4482 12 5.00049 12Z" - fill="#97979B" /> + fill="var(--fgcolor-neutral-tertiary)" /> <path fill-rule="evenodd" clip-rule="evenodd" d="M12.0004 2C12.4542 2 12.851 2.30548 12.9671 2.74411L14.1464 7.19893L17.5002 9.13381C17.8097 9.3124 18.0004 9.64262 18.0004 10C18.0004 10.3574 17.8097 10.6876 17.5002 10.8662L14.1464 12.8011L12.9671 17.2559C12.851 17.6945 12.4542 18 12.0004 18C11.5467 18 11.1498 17.6945 11.0337 17.2559L9.85451 12.8011L6.50076 10.8662C6.19121 10.6876 6.00049 10.3574 6.00049 10C6.00049 9.64262 6.19121 9.31241 6.50076 9.13382L9.85451 7.19893L11.0337 2.74411C11.1498 2.30548 11.5467 2 12.0004 2Z" - fill="#97979B" /> + fill="var(--fgcolor-neutral-tertiary)" /> </svg>Verify that
--fgcolor-neutral-tertiary(or the appropriate variable from your design system) resolves to the desired color.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/columns.svelte (1)
30-53: Ensure route change settles before reading page data.After await goto(...), consider await tick() before using page.data (for name fallback) to avoid transient stale values post-navigation.
- if (!isOnRowsPage) { - await goto( + if (!isOnRowsPage) { + await goto( resolve( '/(console)/project-[region]-[project]/databases/database-[database]/table-[table]', { region: page.params.region, project: page.params.project, database: page.params.database, table: page.params.table } ) - ); + ); + await tick(); }src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheetCards.svelte (1)
23-31: Prevent clicks when disabled; avoid firing onClick for disabled cards.Currently onClick fires regardless of disabled. Guard it to match visual state and avoid accidental actions.
- on:click={() => onClick?.()}> + on:click={(e) => { + if (disabled) { + e.preventDefault(); + e.stopPropagation(); < 8000 span class="pl-mi1">+ return; + } + onClick?.(); + }}>src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (4)
1012-1013: Throttle window scroll handler to reduce layout thrash.recalcAll is heavy; reuse recalcAllThrottled for window scroll.
-<svelte:window on:resize={recalcAll} on:scroll={recalcAll} on:click={handleGlobalClick} /> +<svelte:window on:resize={recalcAll} on:scroll={recalcAllThrottled} on:click={handleGlobalClick} />
591-595: Also clear selectedColumnName on deselect to avoid stale labels.Prevents stale badge text if UI reopens quickly after a deselect.
function resetSelectedColumn() { selectedColumnId = null; previousColumnId = null; - /*selectedColumnName = null;*/ + selectedColumnName = null; }
542-553: Use rAF instead of setTimeout(0) for mobile overlay updates.rAF aligns DOM writes with paint for smoother visuals.
- async function updateOverlaysForMobile(value: boolean) { + async function updateOverlaysForMobile(value: boolean) { if ($isSmallViewport) { - setTimeout(() => { + requestAnimationFrame(() => { [rangeOverlayEl, fadeBottomOverlayEl].forEach((el) => { if (el) { el.style.opacity = value ? '0' : '1'; } }); - }, 0); + }); } }
472-483: Align min with known backend cap to fill placeholders.You request min: 6 while placeholders are 7 and backend max is 7. Consider min: 7 to fully populate initial slots. Based on learnings.
- min: 6 + min: 7src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
246-253: Add analytics for “Generate random data” to match Clear filters/Import CSVFor parity and funnel tracking, fire a click/submit event when opening the random‑data modal.
Example:
=> { - $randomDataModalState.show = true; + $randomDataModalState.show = true; + trackEvent(Click.DatabaseRandomDataOpen); }} />If
Click.DatabaseRandomDataOpendoesn’t exist, align with your analytics enum naming.Also applies to: 277-284
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte (4)
130-161: Hard-coded 1387px width is brittle; compute filler widthWhen no custom columns,
actions: 1387will break across viewports. Compute from container width and other fixed columns.- let columnWidths = { + const viewportWidth = + spreadsheetContainer?.clientWidth || + (typeof window !== 'undefined' ? window.innerWidth : 0); + + let columnWidths = { id: fixedWidths.id, createdAt: fixedWidths.id, updatedAt: fixedWidths.id, custom: minColumnWidth, - actions: hasCustomColumns ? fixedWidths.actions : 1387 + actions: hasCustomColumns + ? fixedWidths.actions + : Math.max( + minColumnWidth, + viewportWidth - (fixedWidths.id + 2 * minColumnWidth) // $id + created/updated + ) };Also applies to: 200-209
324-332:dynamicOverlayHeightis written but not used; apply or removeYou set
style:--dynamic-overlay-heightbut the CSS never reads it. Either apply it or drop the state to reduce complexity.Option A — apply:
.spreadsheet-fade-bottom { + height: var(--dynamic-overlay-height, 60.5vh); right: 0; bottom: 0; position: fixed;Option B — remove the state and related code if bottom anchoring is sufficient.
Also applies to: 414-435
284-305: Remove unreachable nestedcolumn.isActionbranchInside the non‑action branch,
column.isActionis always false. Drop the inner check for clarity.- {#if column.isAction} - <Button.Button icon variant="extra-compact"> - <Icon icon={IconPlus} color="--fgcolor-neutral-primary" /> - </Button.Button> - {:else if column.id === 'actions' || column.id === 'empty'} + {#if column.id === 'actions' || column.id === 'empty'} {column.title} {:else} <Layout.Stack
86-89: Avoid magic number 89px for collapsed tabs
89pxis fragile. Source this from a CSS var or compute from the header element height.Example:
- if (!$expandTabs) { - dynamicOverlayHeight = `calc(${dynamicHeight}px - 89px)`; - } + if (!$expandTabs && headerElement) { + const headerExtra = headerElement.offsetHeight ?? 0; + dynamicOverlayHeight = `calc(${dynamicHeight}px - ${headerExtra}px)`; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
package.json(1 hunks)src/lib/components/card.svelte(2 hunks)src/lib/elements/forms/inputLine.svelte(4 hunks)src/lib/elements/forms/inputPoint.svelte(3 hunks)src/lib/elements/forms/inputPolygon.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/columns.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(32 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/icon/aiForButton.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/store.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/boolean.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/datetime.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/email.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/enum.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/float.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/integer.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/ip.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/relationship.svelte(8 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/string.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/url.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte(9 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheetCards.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/indexes/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/columns.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte
📚 Learning: 2025-10-04T11:46:32.504Z
Learnt from: ItzNotABug
PR: appwrite/console#2442
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/float.svelte:88-88
Timestamp: 2025-10-04T11:46:32.504Z
Learning: In the Appwrite Console codebase, for float/double column inputs in database table column configuration files (like float.svelte), use step={0.1} for InputNumber components, not step="any". This is the established pattern for float/double precision inputs.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/float.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: build
- GitHub Check: e2e
🔇 Additional comments (55)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/ip.svelte (1)
42-42: LGTM! Clean implementation of the disabled prop.The disabled prop is correctly exported and consistently propagated to all three form controls (InputText and both Selector.Checkbox components). The logic properly extends existing disabled conditions without disruption, maintaining the component's existing behavior while enabling external control of interactivity.
Also applies to: 77-77, 85-85, 93-93
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/boolean.svelte (1)
42-102: LGTM! Disabled prop correctly propagated.The disabled prop is properly introduced and threaded through to all relevant UI controls (default value selector, required checkbox, and array checkbox). The logic correctly combines the new prop with existing disabled conditions using OR operations, ensuring that when
disabledis true, all controls are appropriately disabled without breaking existing behavior.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
863-863: LGTM! Hover effect added appropriately.The
hoverEffectprop is correctly added to data rows while being intentionally excluded from loading rows (lines 842-855). This follows the existing pattern where interactive features likeshowSelectOnHoverare only applied to actual data rows, not placeholder loading rows.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (2)
46-46: LGTM! Disabled state integration is well-implemented.The addition of the
disabledprop and its propagation to form controls follows the standard pattern. Explicit defaults fordata,editing, anddisabledin the props destructuring improve clarity and prevent undefined values. The disabled state is correctly passed to all relevant UI controls (Required checkbox, Default value checkbox, and InputPoint component) without disrupting existing logic.Also applies to: 50-57, 107-107, 118-118, 139-139
47-47: Line marked as changed but appears unchanged.Line 47 is annotated as modified but shows no visible difference. This may be a whitespace change, formatting adjustment, or annotation inconsistency.
src/lib/elements/forms/inputPoint.svelte (1)
13-13: LGTM! Clean implementation of disabled prop.The
disabledprop is properly defined, destructured, and propagated to both the number inputs and delete button. The delete button correctly combines all disabling conditions.Also applies to: 26-26, 43-43, 51-51
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte (1)
46-46: LGTM! Consistent disabled prop implementation.The
disabledprop is properly integrated throughout the component:
- Correctly defined with a sensible default (
false)- Properly propagated to both checkboxes and the
InputLinecomponent- The destructuring pattern is clean and follows Svelte conventions
Also applies to: 50-54, 114-114, 125-125, 146-146
src/lib/elements/forms/inputPolygon.svelte (1)
20-20: LGTM! Proper disabled prop propagation.The
disabledprop is correctly added and forwarded to allInputLineinstances, enabling unified disable behavior across polygon line inputs.Also applies to: 30-32, 39-39
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (1)
46-46: LGTM! Well-integrated disabled state.The
disabledprop is properly implemented and propagated throughout:
- Correctly defined with a sensible default (
false)- Properly forwarded to both checkboxes and the
InputPolygoncomponent- Follows the same pattern established across other column editors
Also applies to: 50-57, 128-128, 140-140, 161-161
src/lib/elements/forms/inputLine.svelte (1)
17-17: LGTM! Complete disabled prop integration.The
disabledprop is correctly implemented:
- Properly propagated to all
InputPointinstances- Add coordinate button correctly combines both disabling conditions (
nullable || disabled)- Completes the cascading disable behavior through the input component hierarchy
Also applies to: 28-30, 45-45, 58-64
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/enum.svelte (4)
42-45: LGTM! Import additions are properly used.The new imports (
createConservativeandIconInfo) are correctly utilized in the component.
48-48: LGTM! Disabled prop follows Svelte conventions.The new
disabledprop is correctly declared and enables external control of the form's disabled state.
90-102: Verify: Should the Elements input also respect the disabled state?The
InputTagscomponent for "Elements" doesn't receive thedisabledprop, while all other form inputs (default value, required, array) do. Consider whether this is intentional or if the Elements input should also be disabled when the parent component'sdisabledprop is true for consistency.
107-107: LGTM! Disabled logic is correctly implemented.The disabled conditions for each form input properly combine business rules with the component-level
disabledprop:
- Default value disabled when array/required (no default needed) or form disabled
- Required disabled when array (mutually exclusive) or form disabled
- Array disabled when required (mutually exclusive), editing (immutable), or form disabled
Also applies to: 117-117, 125-125
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/float.svelte 10000 (4)
45-46: LGTM! New imports align with UI revamp.The new imports for
createConservative,Layout, andSelectorare properly utilized throughout the component and align with the pink-svelte UI library adoption mentioned in the AI summary.
49-49: LGTM! Disabled prop correctly added to public API.The new
disabledprop with a default value offalseis correctly exposed and aligns with the pattern being applied across all column editors in this PR.
82-111: LGTM! Disabled state correctly propagated to all InputNumber components.The disabled prop is properly threaded through all InputNumber components with appropriate logic:
- Min/Max inputs: simple disabled prop
- Default value input: correctly combines
data.required || data.array || disabledto prevent defaults on required or array fieldsThe
step={0.1}is correctly maintained as per established pattern.Based on learnings
113-127: LGTM! Disabled state correctly propagated to checkboxes with proper business rules.The disabled prop is appropriately integrated into both checkboxes while maintaining existing business rules:
- Required checkbox:
disabled={data.array || disabled}- prevents required when array is true- Array checkbox:
disabled={data.required || editing || disabled}- prevents array when required is true or in edit modeThe mutual exclusion between required and array fields is correctly preserved.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/email.svelte (4)
41-42: LGTM: New imports support disabled state management.The
Selectorcomponent from the external UI library andcreateConservativehelper are appropriately imported to support the new disabled prop functionality and reactive state tracking.
45-45: LGTM: Disabled prop correctly added to public interface.The
disabledprop with a default value offalseprovides parent components control over the form's interactive state.
59-70: LGTM: Reactive state management correctly handles default value logic.The
createConservativehelper appropriately tracksrequiredandarraystate changes to triggerhandleDefaultState, which correctly clears the default value when either condition is true (required fields must be provided, array fields default to empty arrays).
72-94: LGTM: Disabled logic correctly implements mutual exclusion and prop threading.The disabled state is properly threaded through all form inputs with correct business logic:
- InputEmail: Disabled when a default value doesn't apply (required or array fields)
- Required/Array checkboxes: Mutual exclusion prevents both being true simultaneously
- Array checkbox: Additionally disabled during editing (array type can't change post-creation)
- All inputs respect the parent's
disabledpropsrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/string.svelte (2)
58-58: LGTM! Disabled prop correctly propagated through form controls.The new
disabledprop is properly integrated with existing disabled logic across all form inputs and checkboxes. The OR combination ensures the component respects both the global disabled state and field-specific constraints.Also applies to: 92-92, 107-107, 115-115, 123-123, 137-137
129-130: LGTM! Container styling properly updated for disabled state.The cursor and CSS class bindings on the popover holder correctly incorporate the new
disabledprop, ensuring appropriate visual feedback when the encrypt controls are disabled.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/integer.svelte (4)
45-46: LGTM! Imports are properly utilized.The new imports are correctly integrated:
createConservativeis used for reactive store management (lines 69-77), and the UI components are used for layout and form controls throughout the template.
49-49: LGTM! Disabled prop properly introduced.The new
disabledprop is correctly typed and defaulted tofalse, consistent with the broader pattern of centralized disablement across column editors mentioned in the AI summary.
82-98: LGTM! Disabled state properly propagated to min/max inputs.The
disabledprop is correctly passed to bothInputNumbercomponents, ensuring consistent behavior when the component is in a disabled state.
110-124: LGTM! Checkbox disabled states properly configured.Both checkboxes correctly combine business rules with the component-level
disabledprop:
- Required checkbox (line 115): Disabled when
array=trueor the component is disabled- Array checkbox (line 123): Disabled when
required=true, in editing mode, or the component is disabledThe logic properly prevents conflicting states while respecting the disabled prop.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/url.svelte (3)
41-42: LGTM! Imports are used correctly.Both imports are actively utilized:
Selectorfor the checkbox components (lines 80, 88)createConservativefor reactive store management (lines 59-66)
45-46: LGTM! Disabled prop correctly added.The
disabledprop with a sensible default aligns with the broader pattern of disabled state propagation across column editors mentioned in the PR summary. The property is properly exported and consistently propagated to child components.
77-93: LGTM! Disabled logic is correctly implemented.The
disabledprop is appropriately propagated to all interactive elements with sound logic:
- InputURL (line 77): Disabled when
required(no default for required fields),array(constraint), or globally disabled- Required checkbox (line 85): Disabled when
array(mutual exclusivity) or globally disabled- Array checkbox (line 93): Disabled when
required(mutual exclusivity),editing(immutability after creation), or globally disabledThe mutual exclusivity between
requiredandarray, combined with immutability of the array property during editing, is correctly enforced.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/datetime.svelte (3)
45-46: LGTM! Clean import additions.The new imports are properly utilized:
Selectorfor the checkbox components andcreateConservativefor reactive state management.
77-98: LGTM! Disabled logic properly implements business rules.The disabled prop is correctly threaded through all UI controls with appropriate business logic:
- Default value disabled when required, array, or globally disabled
- Required checkbox disabled when array or globally disabled
- Array checkbox disabled when required, editing, or globally disabled
The implementation correctly prevents invalid state combinations.
49-49: No action required—disabled prop is correctly implemented.The
disabledprop is properly declared with a sensible default and consistently applied throughout the component. All column editor components follow the same pattern (integer, float, string, boolean, etc.), suggesting this is an established convention in the codebase. The prop correctly controls disabled state on form elements.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/relationship.svelte (2)
72-72: LGTM! Disabled prop correctly added.The new
disabledprop follows the established pattern and allows parent components to control the disabled state of all interactive elements within the relationship column editor.
162-217: Pattern verified—no changes needed. The disabled state strategy is correct and consistent with other column editors.The composite
disabled={editing || disabled}applied to structural fields (way, related table, relationship type) matches the established pattern used for immutable fields across all column editors. Other editors similarly lock the array checkbox withdisabled={data.required || editing || disabled}to prevent structural changes once created. Keys remaining editable with simple{disabled}aligns with how other editors keep value fields editable during edit mode. The approach correctly distinguishes immutable structural properties from editable value properties.package.json (1)
27-29: LGTM! Dependency updates support new UI components.The updated pink-icons-svelte and pink-svelte dependencies align with the new AI suggestions UI, icon components, and form enhancements introduced in this PR.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/indexes.svelte (2)
11-11: LGTM! Simplified imports after removing confirmation flow.The removal of the
Confirmcomponent import aligns with the simplified dismiss flow where users can directly cancel without an additional confirmation step.
355-355: LGTM! Direct dismissal improves UX.The Cancel button now directly invokes
dismissIndexes(), streamlining the user experience by removing an unnecessary confirmation step. The implementation is consistent with the mobile cancel flow at Line 384.src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte (2)
41-42: Verify that the feature is production-ready.The feature flag is hardcoded to
truewith a comment stating "flow isn't complete yet!" This could expose incomplete functionality to users.Consider using an environment variable or feature flag service to control this:
-// flow isn't complete yet! -const isSuggestionsFeatureEnabled = true; +const isSuggestionsFeatureEnabled = import.meta.env.VITE_ENABLE_AI_SUGGESTIONS === 'true';Or if the flow is actually complete, remove the comment to avoid confusion.
189-203: LGTM! AI suggestion integration is well-structured.The inline alert provides a clear call-to-action for users to leverage AI-powered column suggestions. The modal state management correctly hides the create column sheet while showing the suggestions modal.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (2)
293-345: LGTM! Enhanced empty states with AI suggestions.The empty state updates provide users with clear options for both AI-assisted and manual workflows. The dual-action cards (AI suggestions and manual creation) improve discoverability and offer flexibility based on user preference.
The conditional logic properly handles two scenarios:
- No indexes but columns exist → suggest indexes or create manually
- No columns → create columns or use AI suggestions
69-76: Review comment mischaracterizes the width implementation pattern.The column widths are not actually "fixed" as described. The
getColumnWidthfunction (lines 117-122) retrieves saved user preferences and returns the user's resized value if available, otherwise returns the default. Sinceresizable: trueis set on each column, users can resize and persist custom widths via the preferences system. This is a flexible, persistence-based approach—not a transition to truly fixed pixel values. The same pattern is used consistently across other pages (spreadsheet.svelte, columns/+page.svelte), making it an intentional architectural choice.Likely an incorrect or invalid review comment.
src/lib/components/card.svelte (2)
23-31: LGTM! Enhanced prop flexibility.The prop type refinements improve the component API:
isButtonis now a flexible boolean instead of a literalexternalprop enables controlled new-tab behavior for linksAnchorProps.isButtonis now optional, providing more flexibilityThese changes maintain backwards compatibility while expanding the API surface for more use cases.
50-62: LGTM! Clean conditional spread for external links.The use of a conditional spread operator
{...external ? { target: '_blank' } : {}}is an elegant pattern for conditionally applying the target attribute. This ensures links only open in new tabs when explicitly requested via theexternalprop.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/store.ts (2)
6-6: LGTM! Optionalforcefield added to suggestion type.The
force?: booleanfield extendsTableColumnSuggestionswithout breaking existing code. The field likely controls whether to force regeneration of suggestions or bypass certain logic in the AI-driven workflow.
52-52: LGTM! New modal state store.The
showColumnsSuggestionsModalwritable store provides clean state management for the column suggestions modal visibility. This follows the same pattern asshowIndexesSuggestionsat Line 50.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte (2)
10-23: LGTM! Clean API extension for mobile footer actions.The new props enhance the component's flexibility:
mobileFooterChildren: Optional snippet for custom mobile footer contentonChildrenClick: Optional callback for click handlingBoth are properly typed as optional and maintain backwards compatibility.
63-65: LGTM! Footer snippet integration.The footer snippet correctly passes the toggle function to
mobileFooterChildren, maintaining consistency with the pattern used elsewhere in the component. The optional chaining ensures the snippet is only rendered when provided.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte (2)
10-15: Prop addition LGTM.Public prop isModal with default false is clear and typed.
30-36: Modal-aware copy and switch visibility LGTM.Subtitle logic and hiding the switch in modal mode match the intended UX.
Also applies to: 53-61
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/columns.svelte (2)
11-16: Bindable show prop LGTM.Using $bindable for show enables clean two-way binding from layout.
55-62: Modal content and actions LGTM.Input respects modal mode; Cancel fully resets suggestion state.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
68-73: Imports are correctly configured; no tree-shaking issues.The index file properly re-exports all symbols from store.ts via
export * from './store', includingshowColumnsSuggestionsModal. The import statement in +layout.svelte correctly sources from the../(suggestions)aggregator, which chains to the store exports. No mismatches exist.
490-491: Remove this review comment — code is correctly implemented.The binding is properly configured. The
showprop in ColumnsSuggestions is marked with$bindable(), andshowColumnsSuggestionsModalis a writable store. Two-way binding directly to a $-prefixed store value in component props is supported in Svelte 5 when the child prop is declared bindable and the store is writable. Both conditions are met here, so no refactoring is needed.Likely an incorrect or invalid review comment.
...console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
Show resolved
Hide resolved
...console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
Show resolved
Hide resolved
...console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte
Outdated
Show resolved
Hide resolved
...console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte
Outdated
Show resolved
Hide resolved
...roject-[region]-[project]/databases/database-[database]/table-[table]/columns/integer.svelte
Show resolved
Hide resolved
...project-[region]-[project]/databases/database-[database]/table-[table]/columns/string.svelte
Show resolved
Hide resolved
...ject-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte
Show resolved
Hide resolved
update: actions on indexes empty state.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (1)
337-343: Gate "Suggest columns" card to Cloud/feature availability.Similar to the indexes card, this "Suggest columns" card is visible regardless of cloud availability or feature enablement, potentially misleading users in non-cloud environments.
Apply the same gating pattern as recommended for the suggest indexes card:
+ {#if isCloud && $tableColumnSuggestions.enabled} <EmptySheetCards icon={IconAI} title="Suggest columns" subtitle="Use AI to generate columns" => { $showColumnsSuggestionsModal = true; - }} /> + }} /> + {/if}src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
269-276: Gate "Suggest columns" card to Cloud/feature availability.The TODO comment and past review both flag this: the card is always rendered even when suggestions are disabled or unavailable. Users noted this for internal design evaluation but it remains unaddressed.
Wrap the card in a conditional:
+ {#if isCloud && $tableColumnSuggestions.enabled} <EmptySheetCards icon={IconAI} title="Suggest columns" subtitle="Use AI to generate columns" => { $showColumnsSuggestionsModal = true; - }} /> + }} /> + {/if}src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (2)
280-280: Use standard scroll behavior value.
ScrollToOptionsaccepts'auto'or'smooth';'instant'is non-standard and browsers may ignore it.Apply this diff:
directAccessScroller.scrollTo({ left: Math.max(0, scrollLeft), - behavior: 'instant' + behavior: 'auto' });
679-695: Fix type annotation for columnSchema.Line 683 uses
= null(value assignment) instead of| null(type union), which doesn't makecolumnSchemanullable in TypeScript. While the guard at 693-695 prevents the crash mentioned in past reviews, the type is still incorrect.Apply this diff:
function deleteColumn(columnId: string) { if (!columnId) return; let columnIndex = -1; - let columnSchema: SuggestedColumnSchema = null; + let columnSchema: SuggestedColumnSchema | null = null; for (let index = 0; index < customColumns.length; index++) {
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (1)
46-50: Remove unused userColumns prop.The
userColumnsprop is accepted but never used. Line 403 has it commented out, and line 419 includes it only to silence linting. This adds unnecessary API surface.Remove the prop or document its intended future use:
- let { - userColumns = [] - }: { - userColumns?: Column[]; - } = $props();And remove line 419:
- userColumns; /* silences lint check, variable not read */Also applies to: 403-403, 419-419
📜 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]/(suggestions)/columns.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(32 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/store.ts(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/store.ts
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/columns.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/indexes/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.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 (9)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (1)
69-76: LGTM: Column width configuration.The minimum width constraints for the key and type columns are appropriate and improve readability.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (3)
238-252: LGTM: EmptySheetCards integration for rows.The action cards provide clear options for creating rows and generating test data, improving the empty state UX.
216-231: Approve refactored filtered rows empty state.The actions snippet pattern simplifies the component structure and maintains functionality.
208-208: Store initialization confirmed—no issues found.The
tableColumnSuggestionsstore initializesforceasfalse(notundefined/null), making the condition!== truecorrectly allow grid rendering in the initial state while respecting explicitforce: truetoggles.src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (5)
788-796: LGTM: Column key validation.The guard prevents creating columns with invalid/empty keys and provides clear user feedback.
679-724: Approve delete/undo workflow with 10-second timer.The implementation correctly handles column deletion with a timed undo option, stores the index for restoration, and cleans up state appropriately.
285-309: Approve column highlight positioning.The dynamic CSS variable approach for positioning the highlight overlay is clean and integrates well with the spreadsheet layout.
627-677: LGTM: Column visibility and scroll-into-view logic.The visibility checks account for sticky columns and viewport bounds correctly, and the smooth scrolling centers the column appropriately.
1381-1428: Approve changeColumnTypePopover snippet extraction.The snippet pattern reduces duplication and makes the column type selector reusable across header and editor contexts.
...console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
Outdated
Show resolved
Hide resolved
.../project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (1)
341-380: Empty state logic is sound.The second empty state appropriately handles the no-columns scenario with contextual actions. The conditional rendering maintains consistency with the first empty state's cloud/self-hosted pattern.
Note the minor pattern difference: Line 370 uses direct assignment (
$showColumnsSuggestionsModal = true) while line 317 uses.update(() => true). Both are valid Svelte patterns, though using.set(true)at line 317 would be more explicit.Optional: Simplify store update at line 317
Consider using
.set(true)instead of.update(() => true)for clarity:- showIndexesSuggestions.update(() => true); + showIndexesSuggestions.set(true);
📜 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]/+page.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/indexes/+page.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 (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (3)
26-26: LGTM - New imports support AI suggestion features.The new imports are all utilized in the refactored empty states and properly support the AI-driven suggestion workflow.
Also applies to: 41-44
71-78: LGTM - Column width adjustments are consistent.The updated default widths and minimum widths are properly aligned.
295-338: Well done - Cloud availability gating is properly implemented.The "Suggest indexes" card (lines 310-319) is correctly wrapped in an
{#if isCloud}conditional, addressing the concern from the previous review. The empty state gracefully adapts between cloud and self-hosted deployments:
- Cloud: AI suggestions prioritized, documentation linked in subtitle
- Self-hosted: Manual creation prioritized, documentation presented as an action card
The
disabledstates correctly check column availability.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (2)
278-281: Use a standard scroll behavior.ScrollToOptions supports 'auto' or 'smooth'; 'instant' isn't standard and may be ignored.
directAccessScroller.scrollTo({ left: Math.max(0, scrollLeft), - behavior: 'instant' + behavior: 'auto' });
679-724: Fix type annotation forcolumnSchema.Line 683 assigns
nulltocolumnSchemabut the type annotation doesn't includenull, causing a TypeScript error.Apply this diff:
function deleteColumn(columnId: string) { if (!columnId) return; let columnIndex = -1; - let columnSchema: SuggestedColumnSchema = null; + let columnSchema: SuggestedColumnSchema | null = null;
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte (1)
233-247: Fragile CSS selectors and!importantusage reduce maintainability.The styles have several concerns:
- Line 244: The
!importantdeclaration suggests specificity conflicts and makes future styling changes harder.- Line 239: The selector
div:first-child > :nth-child(2)is fragile and tightly coupled to the Alert component's internal DOM structure, which may break with library updates.- Line 243: Targeting
.ai-icon-holder.notificationalso assumes internal component structure.Consider these alternatives:
- Request that the pink-svelte library expose CSS custom properties (CSS variables) for padding, alignment, and icon sizing.
- Use wrapper classes with higher specificity instead of
!important.- If these styles are commonly needed, propose adding props to the Alert.Inline component for better API control.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (1)
951-1001: Consider extracting border management logic.The
$effectblock handles multiple concerns: class cleanup, border hiding, highlight updates, and state tracking. While functionally correct, extracting the border visibility logic into a helper function would improve readability.Example refactor:
function applyColumnBorders(columnId: string) { const allHeaders = Array.from( spreadsheetContainer.querySelectorAll('[role="rowheader"] [role="cell"][data-column-id]') ); const selectedIndex = allHeaders.findIndex( (cell) => cell.getAttribute('data-column-id') === columnId ); // Hide selected column borders const selectedCells = spreadsheetContainer.querySelectorAll( `[role="cell"][data-column-id="${columnId}"]` ); selectedCells.forEach((cell) => cell.classList.add('hide-border')); // Hide previous column borders if (selectedIndex > 0) { const prevColumnId = allHeaders[selectedIndex - 1].getAttribute('data-column-id'); if (prevColumnId) { const previousCells = spreadsheetContainer.querySelectorAll( `[role="cell"][data-column-id="${prevColumnId}"]` ); previousCells.forEach((cell) => cell.classList.add('hide-border')); } } } $effect(() => { if (!spreadsheetContainer) return; // Cleanup spreadsheetContainer.querySelectorAll('[role="cell"].hide-border') .forEach((cell) => cell.classList.remove('hide-border')); if (!selectedColumnId) return; setTimeout(() => { applyColumnBorders(selectedColumnId); updateColumnHighlight(); if (previousColumnId !== null) { previousColumnId = selectedColumnId; } else { setTimeout(() => (previousColumnId = selectedColumnId), 25); } }, 300); });
📜 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]/(suggestions)/empty.svelte(32 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/(suggestions)/empty.svelte
🔇 Additional comments (8)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte (2)
5-5: LGTM: Import additions support the new AI suggestions feature.The new imports are appropriate and properly typed. All imported symbols are utilized in the component.
Also applies to: 14-17
187-201: Verify the store structure and modal toggling behavior.The implementation assumes
showCreateColumnSheethas ashowproperty (line 196), but the import on line 16 doesn't reveal its structure. Please verify that the store is structured correctly to support$showCreateColumnSheet.show = false.Additionally, consider these improvements:
- Text punctuation: Add a period at the end of the message for consistency (line 198).
- Modal timing: When rapidly toggling between modals, ensure there are no visual glitches or race conditions.
Run the following script to verify the store structure:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (6)
788-796: Good defensive validation.The empty key validation prevents invalid column creation attempts and provides clear user feedback. This is a solid guard against API errors.
1015-1015: Consider performance impact of global click handler.The global
windowclick handler on line 1015 fires on every click in the application. While the implementation is efficient (early DOM traversal check), verify that this doesn't cause noticeable performance issues, especially on slower devices or complex UIs.Alternative: Consider using a click handler on a wrapping element or implementing event delegation if performance becomes a concern.
Also applies to: 311-316
1266-1325: Well-implemented undo/delete UI with good safeguards.The conditional logic correctly handles both selection and undo-delete modes. The guard preventing deletion of the last column (lines 1306-1307) is good UX design, ensuring users always have at least one column.
1381-1428: Excellent refactor using Svelte snippets.Extracting the column type popover into a reusable snippet (used on lines 1100 and 1128) eliminates duplication and improves maintainability. This follows the DRY principle effectively.
1463-1486: Smooth and well-structured animations.The column highlight overlay and sequential column animations use CSS custom properties effectively and employ smooth easing functions. The
fadeInkeyframe animation and staggered delays create a polished user experience.Also applies to: 1736-1750
1557-1559: Verify browser support forinterpolate-size.The
interpolate-size: allow-keywordsproperty is experimental and has limited browser support. Ensure the UI degrades gracefully in browsers that don't support this feature, or consider using a JavaScript-based height transition as a fallback.
...console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
Show resolved
Hide resolved
...console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (4)
46-51: Remove unused userColumns prop or implement it.The prop is unused and a linter workaround remains. Delete it (or add a TODO if keeping for future).
- let { - userColumns = [] - }: { - userColumns?: Column[]; - } = $props(); @@ - onMount(async () => { - userColumns; /* silences lint check, variable not read */ - + onMount(async () => {Also applies to: 418-421
591-595: Reset selectedColumnName for full cleanup.Uncomment to keep selection state consistent.
- /*selectedColumnName = null;*/ + selectedColumnName = null;
278-281: Use a standard scroll behavior.ScrollToOptions supports 'auto' or 'smooth'; 'instant' isn’t standard.
- directAccessScroller.scrollTo({ - left: Math.max(0, scrollLeft), - behavior: 'instant' - }); + directAccessScroller.scrollTo({ + left: Math.max(0, scrollLeft), + behavior: 'auto' + });
682-694: TS type error: columnSchema declared non-null but assigned null.Use a nullable union to avoid type errors and align with the null checks below.
- let columnSchema: SuggestedColumnSchema = null; + let columnSchema: SuggestedColumnSchema | null = null;src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte (1)
269-281: onclick won’t bind on Svelte components; use on:click (+ a11y label).The handler won’t fire on Button.Button. Switch to on:click and add aria-label.
-<Button.Button - icon - variant="extra-compact" - => { +<Button.Button + icon + variant="extra-compact" + aria-label="Create column" + on:click={() => { if (mode === 'rows') { $showCreateColumnSheet.show = true; $showCreateColumnSheet.title = 'Create column'; $showCreateColumnSheet.columns = $tableColumns; $showCreateColumnSheet.columnsOrder = $columnsOrder; } - }}> + }}>
🧹 Nitpick comments (6)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte (1)
30-36: Consider extracting subtitle logic for readability.The nested ternary is functional but could be slightly more readable. Consider refactoring if this logic becomes more complex in the future.
Example refactor:
const subtitle = $derived.by(() => { - return featureActive - ? isModal - ? 'Use AI to suggest useful columns' - : 'Enable AI to suggest useful columns based on your table name' - : 'Sign up for Cloud to generate columns based on your table name'; + if (!featureActive) { + return 'Sign up for Cloud to generate columns based on your table name'; + } + if (isModal) { + return 'Use AI to suggest useful columns'; + } + return 'Enable AI to suggest useful columns based on your table name'; });src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (1)
300-305: Harden external links opened in a new tab.Add rel="noopener noreferrer" to Link.Anchor when target="_blank" to prevent reverse‑tabnabbing.
- <Link.Anchor - target="_blank" - href="https://appwrite.io/docs/products/databases/tables#indexes"> + <Link.Anchor + target="_blank" + rel="noopener noreferrer" + href="https://appwrite.io/docs/products/databases/tables#indexes">Also applies to: 346-351
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
266-268: Add rel to external docs link.Open-in-new-tab links should include rel="noopener noreferrer".
- <Link.Anchor - target="_blank" - href="https://appwrite.io/docs/products/databases"> + <Link.Anchor + target="_blank" + rel="noopener noreferrer" + href="https://appwrite.io/docs/products/databases">src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte (1)
288-305: Remove unreachable inner check for column.isAction.Inside the else-branch (already not action), the inner {#if column.isAction} can’t run. Simplify header cell rendering.
- {#if column.isAction} - <Button.Button icon variant="extra-compact"> - <Icon icon={IconPlus} color="--fgcolor-neutral-primary" /> - </Button.Button> - {:else if column.id === 'actions' || column.id === 'empty'} + {#if column.id === 'actions' || column.id === 'empty'} {column.title} {:else} <Layout.Stack gap="xs" direction="row" alignItems="center" alignContent="center"> {column.title} <SortButton disabled column={column.id} /> </Layout.Stack> {/if}src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (2)
1347-1362: Disable Delete when nothing is selected.Avoid a no-op click by also disabling when no selection and not in undo mode.
- <Button.Button + <Button.Button size="xs" variant="secondary" - disabled={!isUndoDeleteMode && - customColumns.filter((col) => !col.isPlaceholder).length <= 1} + disabled={!isUndoDeleteMode && + ( !selectedColumnId || + customColumns.filter((col) => !col.isPlaceholder).length <= 1)} on:click={() => { if (isUndoDeleteMode) { undoDelete(); } else { deleteColumn(selectedColumnId); } }}>
1089-1091: Add aria-labels to icon-only buttons.Improve accessibility for screen readers.
-<Button.Button icon variant="extra-compact"> +<Button.Button icon variant="extra-compact" aria-label="Add column"> @@ - <Button.Button + <Button.Button size="xs" variant="extra-compact" + aria-label="Change column type" disabled={!isColumnInteractable} on:click={(event) => {Also applies to: 1429-1438
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(34 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte(5 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/indexes/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte
📚 Learning: 2025-09-25T04:31:05.213Z
Learnt from: ItzNotABug
PR: appwrite/console#2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/options.svelte:33-42
Timestamp: 2025-09-25T04:31:05.213Z
Learning: Svelte 5 uses `onclick` instead of `on:click` for event handlers. Event handlers are now properties like any other rather than directives that require the `on:` prefix.
Applied to files:
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/layout/emptySheet.svelte
🔇 Additional comments (7)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/input.svelte (4)
10-14: LGTM! Props declaration is clean.The
isModalprop is properly typed and uses the correct Svelte 5$props()syntax with a sensible default value.
71-81: LGTM! Smooth transition for context input.The wrapper div with slide transition provides a polished UX when revealing the context input. The 200ms duration is appropriate and the guards ensure the input only appears when suggestions are active.
90-92: LGTM! Proper styling for context input.The CSS correctly uses
:global()to target the child input component and applies a background color from the design system.
53-60: Code changes approved. The modal initialization correctly ensures suggestions are enabled.When the modal opens,
triggerColumnSuggestions()incolumns.svelte(line 46) explicitly sets$tableColumnSuggestions.enabled = truebefore rendering the input component. This is further backed up by theonMounthook ininput.svelte(line 18). In modal mode (isModal=true), the switch is intentionally hidden (line 53), but the context input properly appears because it checks$tableColumnSuggestions.enabled && featureActive(line 72).src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte (1)
309-319: Gating for “Suggest indexes” looks good.Card is correctly wrapped in isCloud and disabled when no columns. LGTM.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
208-208: Correct guard on suggestions “force”.Using $tableColumnSuggestions.force !== true prevents accidental hiding when undefined. Nice.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (1)
1252-1260: Svelte component events vs DOM events (FYI).onclick on a native button is fine in Svelte 5; keep as-is. If this becomes a component later, switch to on:click.
...console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (3)
46-50: Remove unuseduserColumnsprop or implement its usage.The
userColumnsprop remains unused. Line 419 contains a linter workaround, and line 403 shows commented-out intended usage. This is dead code that should be removed or properly implemented.
279-281: Use a standard scroll behavior.
behavior: 'instant'isn't standard in ScrollToOptions. Use'auto'for immediate scroll or'smooth'for animated scroll.
591-595: UncommentselectedColumnNamereset for complete state cleanup.
selectedColumnNameis set in multiple places but isn't reset inresetSelectedColumn()due to line 594 being commented out. This creates inconsistent state management.
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (3)
679-724: Minor type annotation improvement.The function properly guards against undefined
columnSchemawith the check at line 693. However, the type annotation at line 683 should beSuggestedColumnSchema | nullinstead ofSuggestedColumnSchema = nullfor correctness.Apply this diff:
- let columnSchema: SuggestedColumnSchema = null; + let columnSchema: SuggestedColumnSchema | null = null;
937-949: Consider renaming function for clarity.The function is named
selectColumnWithIdbut actually takes aColumnobject as a parameter. Consider renaming toselectColumnfor better clarity.Apply this diff:
- function selectColumnWithId(column: Column) { + function selectColumn(column: Column) {And update the call site at line 1257:
- selectColumnWithId(column); + selectColumn(column);
1298-1366: LGTM: Enhanced action bar provides clear user feedback.The floating action bar effectively communicates selection and deletion states. The guard preventing deletion of the last column is a good safeguard.
Consider extracting this into a separate component if the file continues to grow, as it's a self-contained UI element with its own logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(34 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/(suggestions)/empty.svelte
🔇 Additional comments (14)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (14)
69-77: LGTM: Well-structured state for column selection.The new state variables for column selection and undo functionality are properly typed and initialized.
285-309: LGTM: Highlight positioning logic is sound.The
updateColumnHighlightfunction correctly calculates overlay position using DOM measurements and CSS custom properties.
311-316: LGTM: Outside-click deselection is correctly implemented.The global click handler appropriately resets selection when clicking outside the spreadsheet area.
726-757: LGTM: Undo logic is correctly implemented.The
undoDeletefunction properly restores columns with fallback handling and cleanup.
788-796: LGTM: Column key validation prevents invalid submissions.The validation check ensures columns have valid keys before creation, with appropriate user feedback.
951-1001: LGTM: Selection effect manages visual feedback correctly.The reactive effect properly handles border visibility and highlight positioning. The timing delays (300ms, 25ms) align with animation transitions.
1047-1047: LGTM: Global event handlers are properly attached.Window event handlers for resize, scroll, and click are correctly implemented. Svelte automatically cleans these up on component destruction.
1065-1075: LGTM: Highlight overlay implementation is clean.The highlight overlay with conditional rendering and transitions provides smooth visual feedback.
1144-1151: LGTM: Focus handlers maintain consistent visual state.The focus handlers correctly manage border visibility during cell editing.
1228-1239: LGTM: Mobile footer provides appropriate actions.The mobile-specific delete button is properly positioned and wired.
1245-1263: LGTM: Row rendering enables column selection.The column selector buttons provide an intuitive way to select columns for editing. Proper guards prevent interaction with placeholders and on tablet viewports.
1422-1469: LGTM: Column type popover is well-structured.The popover snippet provides a clean interface for changing column types with proper state management.
1523-1562: LGTM: CSS styles support smooth animations.The styles for column highlighting and selection provide polished visual feedback with appropriate transitions.
1616-1680: LGTM: Floating action wrapper styles are responsive and feature-rich.The styles handle multiple states and screen sizes effectively. The
interpolate-sizeproperty enables smooth transitions, and the comment acknowledges its limited but growing browser support.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (1)
287-296: Gate the "Suggest columns" card withcanShowSuggestionsSheetto prevent dead-end clicks.The card is currently only gated by
isCloud, but suggestions can be unavailable even on cloud (e.g., when disabled or table mismatch). Line 268 correctly usesisCloud && canShowSuggestionsSheetfor the suggestions sheet. Apply the same guard here to prevent opening a modal when the feature is unavailable.Based on learnings
Apply this diff:
{#snippet actions()} - {#if isCloud} + {#if isCloud && canShowSuggestionsSheet} <!-- shown on cloud --> <EmptySheetCards icon={IconAI} title="Suggest columns" subtitle="Use AI to generate columns" => { $showColumnsSuggestionsModal = true; }} /> {/if}src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (4)
46-50: Remove unuseduserColumnsprop or implement its usage.The
userColumnsprop is declared but never used. Line 419 contains a workaround comment to silence the linter, and line 403 shows where it was intended to be used but is commented out.
279-281: Use a standard scroll behavior.ScrollToOptions supports 'auto' or 'smooth'; 'instant' isn't standard and may be ignored by browsers.
591-595: UncommentselectedColumnNamereset for complete state cleanup.The
selectedColumnNameis set in multiple places (lines 754, 940) but isn't reset inresetSelectedColumn()due to being commented out on line 594.
679-724: Fix type declaration:columnSchemashould be nullable.Line 683 declares
columnSchemaasSuggestedColumnSchemabut assignsnullto it. This is a type error that could lead to runtime issues.Apply this diff:
function deleteColumn(columnId: string) { if (!columnId) return; let columnIndex = -1; - let columnSchema: SuggestedColumnSchema = null; + let columnSchema: SuggestedColumnSchema | null = null;
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (3)
542-552: Consider usingrequestAnimationFramefor visual updates.The
setTimeout(() => {...}, 0)pattern works butrequestAnimationFrameis more appropriate for DOM visual updates as it's synchronized with the browser's repaint cycle.Apply this diff:
async function updateOverlaysForMobile(value: boolean) { if ($isSmallViewport) { - setTimeout(() => { + requestAnimationFrame(() => { [rangeOverlayEl, fadeBottomOverlayEl].forEach((el) => { if (el) { el.style.opacity = value ? '0' : '1'; } }); - }, 0); + }); } }
627-650: Consider extracting magic number to file-level constant.The
STICKY_COLUMN_WIDTH = 40is hardcoded within the function. Since this value represents the width of sticky columns (selection and actions), it could be defined at the file level for better maintainability, especially if this value is used elsewhere or needs to be updated.Extract to file level:
+const STICKY_COLUMN_WIDTH = 40; + const baseColProps = { draggable: false, resizable: false };
1015-1045: Remove commented-out countdown progress code.This large block of commented-out SVG countdown timer code is not used anywhere. If it's not planned for near-term implementation, it should be removed to reduce clutter. If it is planned, add a TODO comment explaining when/why it will be restored.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(35 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte(4 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
🔇 Additional comments (19)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+page.svelte (6)
9-9: LGTM!The new imports are properly used throughout the file for the revamped empty state UI and suggestions feature.
Also applies to: 29-47
162-204: LGTM!The layout restructuring properly organizes the action buttons with appropriate nesting and alignment.
220-220: LGTM!The force guard now correctly uses
!== trueto handle undefined/null values, ensuring the spreadsheet renders when suggestions are not forced.
228-266: LGTM!The snippet-based actions properly integrate with EmptySheet components, using modern Svelte 5 syntax and the new EmptySheetCards for a card-based UI.
268-269: LGTM!The suggestions empty sheet is properly gated with both
isCloudandcanShowSuggestionsSheet, ensuring it only renders when the feature is available.
271-286: LGTM!The empty columns state properly differentiates between cloud and self-hosted environments, providing appropriate documentation links and action cards for each context.
Also applies to: 298-323
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (13)
285-309: LGTM: Column highlighting positioning logic.The
updateColumnHighlightfunction correctly calculates and applies the position of the column highlight overlay using CSS custom properties.
311-316: LGTM: Click outside handler.The
handleGlobalClickfunction properly resets the column selection when clicking outside the spreadsheet, which is good UX.
726-757: LGTM: Undo deletion implementation.The
undoDeletefunction correctly restores the deleted column at its original index with proper fallback handling, clears the undo state and timer, and triggers the necessary view recalculation.
788-796: LGTM: Column key validation.The validation check for empty column keys is a good addition that prevents invalid columns from being created and provides clear feedback to the user.
937-949: LGTM: Column selection with auto-scroll.The
selectColumnWithIdfunction properly handles visibility checks and scrolls the column into view when necessary, providing smooth UX.
951-1001: LGTM: Selection state effect with good cleanup.The
$effectcorrectly manages the column selection borders and highlighting. The cleanup of previous borders and the timing delays are well-coordinated with the UI animations.
1083-1084: Verify intentional empty handler forbottomActionClick.The
bottomActionClickprop is set to an empty function. Please confirm this is intentional to disable the bottom action functionality, or if this was left as a placeholder that should be implemented.
1144-1171: LGTM: Cell editor focus handling.The focus handlers properly manage the selection state and border visibility during column editing, providing a clean UX.
1228-1239: LGTM: Mobile footer delete action.The mobile footer snippet properly implements the delete action for mobile viewports with appropriate styling and event handling.
1245-1263: LGTM: Column selector buttons for better UX.The addition of invisible buttons in empty cells provides a better touch target for column selection, especially on mobile devices.
1300-1368: LGTM: Undo/delete floating action bar.The conditional rendering and state management for the undo/delete action bar is well-implemented, providing clear feedback to users about the current operation.
1424-1471: LGTM: Column type popover snippet.The
changeColumnTypePopoversnippet is well-structured and provides a clean way to change column types with proper icon display and event handling.
1710-1746: LGTM: Fade overlay styling.The dual fade overlay system (snow and spreadsheet) is well-implemented with proper dark theme support and good visual polish.
...console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (1)
1591-1596: Normalize CSS token names (case‑sensitive custom properties).Use lowercase tokens for consistency with the rest of the file.
- border-radius: var(--border-radius-S, 8px); + border-radius: var(--border-radius-s, 8px); ... - border: var(--border-width-L, 2px) solid rgba(253, 54, 110, 0.24) !important; + border: var(--border-width-l, 2px) solid rgba(253, 54, 110, 0.24) !important;Also applies to: 1794-1802
♻️ Duplicate comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (4)
279-281: Use a standard scroll behavior.directAccessScroller.scrollTo({ left: Math.max(0, scrollLeft), - behavior: 'instant' + behavior: 'auto' });
46-50: Remove dead userColumns prop and linter silencer (or document future use).- let { - userColumns = [] - }: { - userColumns?: Column[]; - } = $props(); ... - /*...userColumns,*/ ... - userColumns; /* silences lint check, variable not read */Also applies to: 403-403, 419-419
591-595: Reset selectedColumnName for full cleanup.function resetSelectedColumn() { selectedColumnId = null; previousColumnId = null; - /*selectedColumnName = null;*/ + selectedColumnName = null; }
683-694: Type columnSchema as nullable to satisfy TS and match guard.- let columnSchema: SuggestedColumnSchema = null; + let columnSchema: SuggestedColumnSchema | null = null; ... - // store column with its index for undo - columnBeingDeleted = { ...columnSchema, deletedIndex: columnIndex }; + // store column with its index for undo + columnBeingDeleted = { ...columnSchema, deletedIndex: columnIndex };Also applies to: 700-704, 701-702
🧹 Nitpick comments (3)
src/routes/(console 32E9 )/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (3)
1003-1013: Clear pending undo timer on unmount.Avoids timers mutating state after component destroy.
onDestroy(() => { resizeObserver?.disconnect(); hScroller?.removeEventListener('scroll', recalcAllThrottled); if (scrollAnimationFrame) { cancelAnimationFrame(scrollAnimationFrame); } + + if (undoTimer) { + clearTimeout(undoTimer); + undoTimer = null; + } customColumns = []; resetSuggestionsStore(); });
114-118: Clamp overlay height to non‑negative to avoid negative calc values on small viewports.- const dynamicHeightPx = viewportHeight - headerRect.bottom; + const dynamicHeightPx = Math.max(0, viewportHeight - headerRect.bottom);
598-606: Guard against missing container before querying DOM.function handlePreviousColumnsBorder(columnId: string, hide: boolean = true) { + if (!spreadsheetContainer) return; const allHeaders = Array.from( spreadsheetContainer.querySelectorAll( '[role="rowheader"] [role="cell"][data-column-id]' ) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte(35 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
PR: appwrite/console#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]/(suggestions)/empty.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 (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (2)
1621-1623: Good progressive enhancement for interpolate-size.Wrapping in @supports is the right call; degrades cleanly where unsupported.
1065-1075: Nice column highlight overlay implementation.Clean CSS‑var positioning and subtle fade/slide make selection obvious without interfering with interactions.
Also applies to: 1525-1539
| <button | ||
| class="column-selector-button" | ||
| aria-label="Select column" | ||
| onclick={() => { | ||
| if (isColumnInteractable && !$isTabletViewport) { | ||
| selectColumnWithId(column); | ||
| } | ||
| }}></button> | ||
| </Spreadsheet.Cell> | ||
| {/each} |
There was a problem hiding this comment.
Fix Svelte event binding: use on:click (and set button type).
onclick won’t wire in Svelte. This breaks column selection.
Apply:
- <Spreadsheet.Cell {root} column={column.id} isEditable={false}>
- <button
+ <Spreadsheet.Cell {root} column={column.id} isEditable={false}>
+ <button
+ type="button"
class="column-selector-button"
aria-label="Select column"
- => {
+ on:click={() => {
if (isColumnInteractable && !$isTabletViewport) {
selectColumnWithId(column);
}
}}></button>
</Spreadsheet.Cell>📝 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.
| <button | |
| class="column-selector-button" | |
| aria-label="Select column" | |
| => { | |
| if (isColumnInteractable && !$isTabletViewport) { | |
| selectColumnWithId(column); | |
| } | |
| }}></button> | |
| </Spreadsheet.Cell> | |
| {/each} | |
| <button | |
| type="button" | |
| class="column-selector-button" | |
| aria-label="Select column" | |
| on:click={() => { | |
| if (isColumnInteractable && !$isTabletViewport) { | |
| selectColumnWithId(column); | |
| } | |
| }}></button> | |
| </Spreadsheet.Cell> | |
| {/each} |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
around lines 1252-1261, the button uses a plain onclick attribute which Svelte
does not bind; replace onclick with Svelte's on:click and add an explicit
type="button" to prevent accidental form submission, keeping the same handler
logic (check isColumnInteractable and !$isTabletViewport then call
selectColumnWithId(column)).
| {#if !creatingColumns} | ||
| <div class="floating-action-wrapper expanded" class:selection={hasSelection}> | ||
| <FloatingActionBar> | ||
| <svelte:fragment slot="start"> | ||
| <Layout.Stack | ||
| gap="xs" | ||
| direction="row" | ||
| alignItems="center" | ||
| style="padding-bottom: 0;"> | ||
| <Badge | ||
| size="xs" | ||
| variant="secondary" | ||
| content={columnName} | ||
| type={isUndoDeleteMode ? 'error' : undefined} /> | ||
|
|
||
| <Typography.Caption | ||
| variant="400" | ||
| color="--fgcolor-neutral-primary" | ||
| style="white-space: nowrap;"> | ||
| {#if isUndoDeleteMode} | ||
| was deleted. You can undo this action. | ||
| {:else} | ||
| column selected | ||
| {/if} | ||
| </Typography.Caption> | ||
| </Layout.Stack> | ||
| </svelte:fragment> | ||
|
|
||
| <svelte:fragment slot="end"> | ||
| <Layout.Stack direction="row" gap="xs" alignItems="center" inline> | ||
| {#if !isUndoDeleteMode} | ||
| <Button.Button | ||
| size="xs" | ||
| variant="text" | ||
| on:click={() => (selectedColumnId = null)}> | ||
| Cancel | ||
| </Button.Button> | ||
|
|
||
| <!--{:else}--> | ||
| <!-- {@render countdownProgress()}--> | ||
| {/if} | ||
| <Button.Button | ||
| size="xs" | ||
| variant="secondary" | ||
| disabled={!isUndoDeleteMode && | ||
| customColumns.filter((col) => !col.isPlaceholder).length <= 1} | ||
| on:click={() => { | ||
| if (isUndoDeleteMode) { | ||
| undoDelete(); | ||
| } else { | ||
| deleteColumn(selectedColumnId); | ||
| } | ||
| }}> | ||
| {#if isUndoDeleteMode} | ||
| Undo | ||
| {:else} | ||
| Delete | ||
| {/if} | ||
| </Button.Button> | ||
| </Layout.Stack> | ||
| </svelte:fragment> | ||
| </FloatingActionBar> | ||
| </div> | ||
| {/if} | ||
|
|
There was a problem hiding this comment.
Render the Delete/Undo bar only when a column is selected or undo is active.
Currently shows “column selected” with an empty badge and an enabled Delete in some cases; confusing UX.
- {#if !creatingColumns}
- <div class="floating-action-wrapper expanded" class:selection={hasSelection}>
+ {#if !creatingColumns && hasSelection}
+ <div class="floating-action-wrapper expanded" class:selection={true}>
<FloatingActionBar>
...
</FloatingActionBar>
</div>
{/if}🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
around lines 1305 to 1369: the floating Delete/Undo bar is being shown anytime
!creatingColumns, producing an empty badge and enabled Delete when no column is
selected; change the render condition so the bar only mounts when there is an
active selection or an undo state (i.e. selectedColumnId is non-null OR
isUndoDeleteMode is true). Also gate the Badge text and the Canc
7AA9
el/Delete
actions behind the same condition (only show columnName when selectedColumnId
exists), and ensure the Delete button remains disabled when there is no real
selected column (preserve the existing placeholder-count guard). This will
prevent the empty badge and confusing enabled Delete when nothing is selected.
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
Bug Fixes / UX
Chores