-
Notifications
You must be signed in to change notification settings - Fork 177
✨ feat(command palette): support Column options #3732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Finished running flow.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Updates to Preview Branch (feat/table-column-options) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
WalkthroughAdds column-aware table mode to the Command Palette: new input/suggestion type 'column', rendering of per-table column options, column/table previews, and URL navigation to specific columns. Wires an isTableModeActivatable prop through components. Introduces supporting utilities, styles, and comprehensive tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CP as CommandPalette
participant CPC as CommandPaletteContent
participant SI as CommandPaletteSearchInput
participant TCO as TableColumnOptions
participant URL as URL Utils
participant ERD as ERD Renderer
U->>CP: Open palette
CP->>CPC: Render (isTableModeActivatable)
CPC->>SI: Render (table mode activatable)
U->>SI: Type "users n..."
SI-->>CPC: Input change (table mode)
CPC->>TCO: Render options (table + columns)
U->>TCO: Select column "users/name"
TCO->>URL: getTableColumnLinkHref("users","name")
URL-->>TCO: href with ?active=users#users__columns__name
TCO->>ERD: Navigate (select table/column)
ERD-->>U: Focus table/column
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Check changeset necessityStatus: REQUIRED Reason:
Changeset (copy & paste):---
"@liam-hq/erd-core": minor
---
- ✨ Command Palette: add table mode with column options and previews
- Allow switching to a table-specific input mode to list and filter column options
- Keep the selected table option pinned; show table preview when hovering a column
- Support Enter and Cmd/Ctrl+Enter to navigate to tables/columns (with deep links preserving query params)
- Display column type icons (PK, FK, NOT NULL, nullable) in the options list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx (1)
193-209
: Test intent vs. actions mismatch; fix Tab test to truly assert no table-mode switchThe test claims “switch to command” but never presses '>' and doesn’t assert table options are hidden. Tighten it:
Apply this diff:
- // switch to "command" input mode - await user.keyboard('{Tab}') + // switch to "command" input mode, then press Tab (should not activate table mode) + await user.keyboard('>') + await user.keyboard('{Tab}') @@ - // command options still be displayed because table mode is not activated + // still in command mode: command options visible, table options hidden expect( screen.getByRole('option', { name: 'Copy Link ⌘ C' }), ).toBeInTheDocument() expect( screen.getByRole('option', { name: 'Zoom to Fit ⇧ 1' }), ).toBeInTheDocument() + expect(screen.queryByRole('option', { name: 'users' })).not.toBeInTheDocument() + expect(screen.queryByRole('option', { name: 'posts' })).not.toBeInTheDocument()
🧹 Nitpick comments (5)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts (1)
13-22
: Consider additional edge case tests.The test correctly verifies that existing query parameters are preserved. However, consider adding test cases for:
- What happens when an
active
parameter already exists (should be overwritten)- Table/column names with special characters that need URL encoding (e.g., spaces, special chars)
- Empty string inputs for tableName or columnName
Example additional test:
it('should overwrite existing active parameter', () => { window.location.search = '?active=posts&page=2' window.location.hash = '' expect(getTableColumnLinkHref('users', 'created_at')).toBe( '?active=users&page=2#users__columns__created_at', ) })frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts (1)
4-12
: Consider input validation.The function logic is correct and URLSearchParams will properly handle URL encoding. However, there's no validation for empty or undefined inputs.
Consider adding input validation:
export const getTableColumnLinkHref = ( activeTableName: string, columnName: string, ) => { + if (!activeTableName || !columnName) { + throw new Error('activeTableName and columnName are required') + } const searchParams = new URLSearchParams(window.location.search) searchParams.set('active' satisfies QueryParam, activeTableName) const targetElementId = getTableColumnElementId(activeTableName, columnName) return `?${searchParams.toString()}#${targetElementId}` }Alternatively, if empty strings are valid in your domain, document this behavior with a JSDoc comment.
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/utils/getColumnTypeMap.ts (1)
5-13
: Simplify FK detection and add explicit return typeUse Array.some for clarity and declare the returned map type.
Apply these diffs:
-const isForeignKey = (columnName: string, table: Table): boolean => { - for (const constraint of Object.values(table.constraints)) { - if (constraint.type !== 'FOREIGN KEY') continue - - if (constraint.columnNames.includes(columnName)) return true - } - - return false -} +const isForeignKey = (columnName: string, table: Table): boolean => { + return Object.values(table.constraints).some( + (c) => c.type === 'FOREIGN KEY' && c.columnNames.includes(columnName), + ) +}-export const getColumnTypeMap = (table: Table) => { - const result: Record<string, ColumnType> = {} +export const getColumnTypeMap = (table: Table): Record<string, ColumnType> => { + const result: Record<string, ColumnType> = {}Also applies to: 15-29
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx (2)
67-89
: Prefer scoped handlers: replace document keydown with Command.Item onSelectGlobal keydown listeners are brittle and can leak across components. cmdk provides onSelect at item-level; use it and remove document listeners.
Apply these diffs to remove listeners:
- // Select option by pressing [Enter] key (with/without ⌘ key) - useEffect(() => { - // It doesn't subscribe a keydown event listener if the suggestion type is not "table" - if (suggestion?.type !== 'table') return - - const down = (event: KeyboardEvent) => { - const suggestedTableName = suggestion.name - - if (event.key === 'Enter') { - event.preventDefault() - - if (event.metaKey || event.ctrlKey) { - window.open(getTableLinkHref(suggestedTableName)) - } else { - goToERD(suggestedTableName) - } - } - } - - document.addEventListener('keydown', down) - return () => document.removeEventListener('keydown', down) - }, [suggestion, goToERD]) + // keyboard selection handled via Command.Item onSelect- // Select option by pressing [Enter] key (with/without ⌘ key) - useEffect(() => { - // It doesn't subscribe a keydown event listener if the suggestion type is not "column" - if (suggestion?.type !== 'column') return - - const down = (event: KeyboardEvent) => { - const { tableName, columnName } = suggestion - - if (event.key === 'Enter') { - event.preventDefault() - - if (event.metaKey || event.ctrlKey) { - window.open(getTableColumnLinkHref(tableName, columnName)) - } else { - goToERD(tableName, columnName) - } - } - } - - document.addEventListener('keydown', down) - return () => document.removeEventListener('keydown', down) - }, [suggestion, goToERD]) + // keyboard selection handled via Command.Item onSelectAdd onSelect handlers:
- <Command.Item + <Command.Item value={getSuggestionText({ type: 'table', name: table.name })} + onSelect={() => goToERD(table.name)} >- <Command.Item + <Command.Item key={column.name} value={getSuggestionText({ type: 'column', tableName: table.name, columnName: column.name, })} + onSelect={() => goToERD(table.name, column.name)} >Note: If you need Cmd/Ctrl+Enter to open in a new tab, add onKeyDown on the to handle metaKey + Enter locally instead of global listeners. Based on learnings.
Also applies to: 90-111, 125-145, 146-177
41-47
: Icon accessibility: hide decorative SVGs from screen readersThese icons duplicate the text. Mark them hidden for better a11y.
Apply this diff:
- case 'PRIMARY_KEY': - return <KeyRound {...props} /> + case 'PRIMARY_KEY': + return <KeyRound aria-hidden {...props} /> case 'FOREIGN_KEY': - return <Link {...props} /> + return <Link aria-hidden {...props} /> case 'NOT_NULL': - return <DiamondFillIcon aria-label={undefined} {...props} /> + return <DiamondFillIcon aria-hidden {...props} /> case 'NULLABLE': - return <DiamondIcon aria-label={undefined} {...props} /> + return <DiamondIcon aria-hidden {...props} />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx
(1 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx
(3 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx
(4 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/CommandPaletteOptions.module.css
(1 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx
(1 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/utils/getColumnTypeMap.ts
(1 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts
(1 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.test.ts
(4 hunks)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts
(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts
(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts
(1 hunks)frontend/packages/erd-core/src/features/erd/utils/url/index.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.test.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/utils/getColumnTypeMap.ts
frontend/packages/erd-core/src/features/erd/utils/url/index.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests with filenames ending in .test.ts or .test.tsx colocated near source
Files:
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.test.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}
: Use runtime type validation with valibot for external data validation
Prefer early returns for readability
Write simple, direct code without backward compatibility shims; update all call sites together
Use const-assigned arrow functions instead of function declarations for small utilities (e.g., const toggle = () => {})
Follow existing import patterns and tsconfig path aliases
Files:
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.test.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/utils/getColumnTypeMap.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx
frontend/packages/erd-core/src/features/erd/utils/url/index.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx
frontend/packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Shared libraries and tools live under frontend/packages
Files:
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.test.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/utils/getColumnTypeMap.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx
frontend/packages/erd-core/src/features/erd/utils/url/index.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/CommandPaletteOptions.module.css
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
**/*.tsx
: Prefix React event handler functions with "handle" (e.g., handleClick)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx
**/!(page).tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use named exports only (no default exports) for React/TSX modules
Files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx
**/*.module.css
📄 CodeRabbit inference engine (AGENTS.md)
Use CSS Modules named *.module.css and keep types via typed-css-modules
**/*.module.css
: Use CSS variables from @liam-hq/ui for styling tokens
Use spacing CSS variables only for margins/padding; use size units (rem, px, etc.) for width/height
Files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/CommandPaletteOptions.module.css
**/*.css
📄 CodeRabbit inference engine (CLAUDE.md)
Use CSS Modules for all styling (i.e., prefer *.module.css; avoid global CSS)
Files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/CommandPaletteOptions.module.css
🧠 Learnings (2)
📚 Learning: 2025-08-23T03:36:21.590Z
Learnt from: tnyo43
PR: liam-hq/liam#3129
File: frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx:11-15
Timestamp: 2025-08-23T03:36:21.590Z
Learning: The CommandPalette and its child components (including CommandPaletteContent and TableOptions) in the ERD renderer are only rendered on the client side, so SSR safety checks for window object access are not necessary in this context.
Applied to files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx
📚 Learning: 2025-08-23T03:36:21.590Z
Learnt from: tnyo43
PR: liam-hq/liam#3129
File: frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableOptions.tsx:11-15
Timestamp: 2025-08-23T03:36:21.590Z
Learning: In the liam ERD codebase, CommandPalette.tsx has a 'use client' directive, ensuring that all CommandPalette child components (CommandPaletteContent, TableOptions, etc.) run only on the client side, making window object access safe without additional typeof checks.
Applied to files:
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx
🧬 Code graph analysis (9)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts (1)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts (1)
getTableColumnLinkHref
(4-12)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts (1)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts (1)
getTableColumnElementId
(3-6)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.test.ts (2)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts (1)
CommandPaletteSuggestion
(6-9)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts (2)
getSuggestionText
(5-11)textToSuggestion
(13-32)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/utils/getColumnTypeMap.ts (1)
frontend/packages/schema/src/index.ts (1)
isPrimaryKey
(69-69)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx (5)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts (1)
textToSuggestion
(13-32)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts (1)
CommandPaletteInputMode
(1-4)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteSearchInput/CommandPaletteSearchInput.tsx (1)
CommandPaletteSearchInput
(26-105)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx (1)
TableColumnOptions
(51-180)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalettePreview/TablePreview.tsx (1)
TablePreview
(10-44)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx (1)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx (1)
CommandPaletteContent
(51-130)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts (1)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts (1)
CommandPaletteSuggestion
(6-9)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx (1)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx (1)
CommandPaletteContent
(51-130)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/TableColumnOptions.tsx (7)
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts (1)
CommandPaletteSuggestion
(6-9)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/utils/getColumnTypeMap.ts (2)
ColumnType
(3-3)getColumnTypeMap
(15-29)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteProvider/hooks.ts (1)
useCommandPaletteOrThrow
(18-23)frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnElementId.ts (1)
getTableColumnElementId
(3-6)frontend/packages/erd-core/src/features/erd/utils/url/getTableLinkHref.ts (1)
getTableLinkHref
(3-7)frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.ts (1)
getTableColumnLinkHref
(4-12)frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts (1)
getSuggestionText
(5-11)
⏰ 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). (4)
- GitHub Check: frontend-ci
- GitHub Check: frontend-lint
- GitHub Check: security-review
- GitHub Check: Supabase Preview
🔇 Additional comments (15)
frontend/packages/erd-core/src/features/erd/utils/url/getTableColumnLinkHref.test.ts (1)
4-11
: LGTM! Basic test case is correct.The test verifies the default behavior when location has no existing parameters. The function correctly builds the query string with the active parameter and hash fragment.
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPalette.tsx (1)
45-45
: LGTM! Prop wiring is correct.The
isTableModeActivatable
prop is correctly passed toCommandPaletteContent
, enabling table mode activation in the CommandPalette. This aligns with the feature's objective to support column options.frontend/packages/erd-core/src/features/erd/utils/url/index.ts (1)
2-2
: LGTM! Export pattern is consistent.The re-export follows the existing pattern and makes the new
getTableColumnLinkHref
utility available through the url utils module.frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteOptions/CommandPaletteOptions.module.css (1)
9-22
: LGTM! Indent styling is well-implemented.The CSS creates a clear visual hierarchy for nested items:
- Extra left padding (var(--spacing-6)) provides room for the guide line
- The
:before
pseudo-element draws a vertical line using 1px width- Color and spacing variables follow the coding guidelines correctly
As per coding guidelines: "Use CSS variables from @liam-hq/ui for styling tokens" and "Use spacing CSS variables only for margins/padding".
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.ts (2)
5-11
: LGTM! Column type handling is correct.The function properly handles the new column suggestion type by returning a three-part string format while preserving existing behavior for table and command types.
13-32
: Confirm separator safety
- Search found no literal
|
in anytableName
orcolumnName
definitions.- If table/column names originate at runtime (e.g., from the database or user input), ensure they can’t include
|
or switch to encoding/using a delimiter guaranteed invalid in identifiers.frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/types.ts (1)
9-9
: LGTM! Type definition is correct.The new column variant properly extends the discriminated union with both
tableName
andcolumnName
fields, maintaining consistency with the existing type structure.frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/utils/suggestion.test.ts (4)
28-38
: LGTM! Column suggestion test is correct.The test properly verifies that
getSuggestionText
returns the expected three-part format for column suggestions.
58-68
: LGTM! Parsing test is comprehensive.The test correctly verifies that
textToSuggestion
parses the three-part column format into the expected object structure.
87-93
: LGTM! Edge case handling is correct.The test verifies that empty column names are properly rejected, ensuring robust parsing behavior.
102-102
: LGTM! Integration test coverage is complete.The addition of the column case ensures round-trip consistency between
getSuggestionText
andtextToSuggestion
for all suggestion types.frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.test.tsx (3)
20-53
: Solid schema fixture for column-aware testsExplicit columns enable column suggestions/previews and keep tests realistic. LGTM.
211-244
: Good coverage: table mode renders table + filtered columnsValidates table-on-top and column filtering behavior. LGTM.
263-284
: Preview behavior for column selections looks correctHovering column options renders the parent table preview as intended. LGTM.
frontend/packages/erd-core/src/features/erd/components/ERDRenderer/CommandPalette/CommandPaletteContent/CommandPaletteContent.tsx (1)
16-29
: Filters, table-mode wiring, and previews look coherent
- default vs. table filters correctly scope items.
- isTableModeActivatable is propagated to input.
- Column suggestion shows table preview as expected.
LGTM.
Also applies to: 31-45, 47-66, 73-87, 103-108, 120-122
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a very similar logic here. However, the way of checking "foreign key" is different approach, so I think it's not possible to reuse it here.
Maybe we can refactor them and integrate their logic into a same one, but let's make it as another issue.
// Select option by pressing [Enter] key (with/without ⌘ key) | ||
useEffect(() => { | ||
// It doesn't subscribe a keydown event listener if the suggestion type is not "table" | ||
if (suggestion?.type !== 'table') return | ||
|
||
const down = (event: KeyboardEvent) => { | ||
const suggestedTableName = suggestion.name | ||
|
||
if (event.key === 'Enter') { | ||
event.preventDefault() | ||
|
||
if (event.metaKey || event.ctrlKey) { | ||
window.open(getTableLinkHref(suggestedTableName)) | ||
} else { | ||
goToERD(suggestedTableName) | ||
} | ||
} | ||
} | ||
|
||
document.addEventListener('keydown', down) | ||
return () => document.removeEventListener('keydown', down) | ||
}, [suggestion, goToERD]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same logic with the TableOption component. I've just hard copied to reduce diffs of this PR. I'll work on refactoring after this PR is merged and remove duplicated code.
This reverts commit 04295f9.
} | ||
|
||
export const CommandPaletteContent: FC<Props> = ({ | ||
isTableModeActivatable = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isTableModeActivatable
will be false
by default, so it won't be displayed for the users yet.
const ColumnIcon: FC<ComponentProps<'svg'> & { columnType: ColumnType }> = ({ | ||
columnType, | ||
...props | ||
}) => { | ||
switch (columnType) { | ||
case 'PRIMARY_KEY': | ||
return <KeyRound {...props} /> | ||
case 'FOREIGN_KEY': | ||
return <Link {...props} /> | ||
case 'NOT_NULL': | ||
return <DiamondFillIcon aria-label={undefined} {...props} /> | ||
case 'NULLABLE': | ||
return <DiamondIcon aria-label={undefined} {...props} /> | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THIS IS GREAT!!!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻 I understand that no changeset was created because there is no user impact.
E2E test is failed now, I'll merge if passed!
const ColumnIcon: FC<ComponentProps<'svg'> & { columnType: ColumnType }> = ({ | ||
columnType, | ||
...props | ||
}) => { | ||
switch (columnType) { | ||
case 'PRIMARY_KEY': | ||
return <KeyRound {...props} /> | ||
case 'FOREIGN_KEY': | ||
return <Link {...props} /> | ||
case 'NOT_NULL': | ||
return <DiamondFillIcon aria-label={undefined} {...props} /> | ||
case 'NULLABLE': | ||
return <DiamondIcon aria-label={undefined} {...props} /> | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THIS IS GREAT!!!!!
Issue
Why is this change needed?
This change will not published yet
I added
TableColumnOptions
to display column options of each table. Users can select table by pressing "Tab" key and see the Column options in the CommandPalette. Selecting column options will openTableDetail
component, so they can access to more information easily.preview: https://liam-oybrx95ix-liambx.vercel.app/erd/p/github.com/mastodon/mastodon/blob/1bc28709ccde4106ab7d654ad5888a14c6bb1724/db/schema.rb
demo
Users can go to the "table" input mode with a specific table by pressing "Tab" key. In the input mode, users can see only the selected table option and its column options.
Screen.Recording.0007-10-09.at.23.25.01.mov
Summary by CodeRabbit
New Features
Style
Tests