-
Notifications
You must be signed in to change notification settings - Fork 188
✨ feat(command palette): Add "Show All" and "Hide All" table visibility shortcuts and options in CommandPalette #4006
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
Changes from all commits
30e0e95
8333c3c
f3235bd
56af585
a435dd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "@liam-hq/erd-core": patch | ||
| --- | ||
|
|
||
| - ✨ Add "Show All Tables" and "Hide All Tables" table visibility shortcuts and options in CommandPalette | ||
| - Command Palette: new "Show All Tables" and "Hide All Tables" commands | ||
| - Subscribe shortcuts: ⇧A for "Show All Tables" and ⇧H for "Hide All Tables" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { renderHook, waitFor } from '@testing-library/react' | ||
| import userEvent from '@testing-library/user-event' | ||
| import { ReactFlowProvider } from '@xyflow/react' | ||
| import { NuqsTestingAdapter, type UrlUpdateEvent } from 'nuqs/adapters/testing' | ||
| import type { ReactNode } from 'react' | ||
| import { afterEach, expect, it, vi } from 'vitest' | ||
| import { UserEditingProvider } from '../../../../../../../stores' | ||
| import { compressToEncodedUriComponent } from '../../../../../../../utils/compressToEncodedUriComponent' | ||
| import { useSubscribeTableVisibility } from './useSubscribeTableVisibility' | ||
|
|
||
| const onUrlUpdate = vi.fn<() => [UrlUpdateEvent]>() | ||
|
|
||
| const wrapper = ({ children }: { children: ReactNode }) => ( | ||
| <NuqsTestingAdapter onUrlUpdate={onUrlUpdate}> | ||
| <ReactFlowProvider | ||
|
Comment on lines
+11
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainFix
Please double‑check the Also applies to: 48-55, 64-71 I'll verify the review comment by examining the test file and understanding the Length of output: 2109 🏁 Script executed: #!/bin/bash
# Search for NuqsTestingAdapter type definitions in the codebase
rg -n "NuqsTestingAdapter" --type=ts --type=tsx -A 5 -B 2Length of output: 27 🏁 Script executed: #!/bin/bash
# Look for onUrlUpdate type definitions or imports
rg -n "onUrlUpdate" --type=ts --type=tsx -C 3Length of output: 27 I can see the test file and the typing issue. Let me search for the NuqsTestingAdapter type definition and verify the expected callback signature: 🏁 Script executed: #!/bin/bash
# Search for NuqsTestingAdapter and onUrlUpdate with corrected rg syntax
rg -n "NuqsTestingAdapter|onUrlUpdate" -A 5 -B 2Length of output: 48526 🏁 Script executed: #!/bin/bash
# Check if nuqs types are available in node_modules or package.json
find . -name "package.json" -type f | head -20 | xargs grep -l "nuqs"Length of output: 41 🏁 Script executed: #!/bin/bash
# Look for type definitions or d.ts files related to nuqs
fd "nuqs" --type f -e d.ts -e ts | grep -E "(nuqs|testing)" | head -20Length of output: 0 🌐 Web query: 💡 Result: Signature (in TypeScript): type UrlUpdateEvent = { type OnUrlUpdateFunction = (event: UrlUpdateEvent) => void; Explanation: the testing adapter calls onUrlUpdate with an object containing the new URLSearchParams, the rendered query string, and the options used for the update (e.g. options.history is 'push' in examples). Sources: [1], [2]. Sources: Fix The mock is typed as const onUrlUpdate = vi.fn<(event: UrlUpdateEvent) => void>()Additionally, the comment at line 64 contradicts the test expectation. The second test shows the hidden list being cleared ( Same typing issue appears in:
🤖 Prompt for AI Agents |
||
| defaultNodes={[ | ||
| { | ||
| id: '1', | ||
| type: 'table', | ||
| data: {}, | ||
| position: { x: 0, y: 0 }, | ||
| hidden: false, | ||
| }, | ||
| { | ||
| id: '2', | ||
| type: 'table', | ||
| data: {}, | ||
| position: { x: 0, y: 0 }, | ||
| hidden: true, | ||
| }, | ||
| ]} | ||
| > | ||
| <UserEditingProvider>{children}</UserEditingProvider> | ||
| </ReactFlowProvider> | ||
| </NuqsTestingAdapter> | ||
| ) | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| it('hides all table nodes by ⇧H', async () => { | ||
| const user = userEvent.setup() | ||
| renderHook(() => useSubscribeTableVisibility(), { wrapper }) | ||
|
|
||
| await user.keyboard('{Shift>}H{/Shift}') | ||
|
|
||
| // hidden query parameter should be added with all node ids | ||
| await waitFor(() => { | ||
| expect(onUrlUpdate).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| queryString: `?hidden=${compressToEncodedUriComponent('1,2')}`, | ||
| }), | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| it('shows all table nodes by ⇧A', async () => { | ||
| const user = userEvent.setup() | ||
| renderHook(() => useSubscribeTableVisibility(), { wrapper }) | ||
|
|
||
| await user.keyboard('{Shift>}A{/Shift}') | ||
|
|
||
| // hidden query parameter should be added with all node ids | ||
| await waitFor(() => { | ||
| expect(onUrlUpdate).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| queryString: '?hidden=', | ||
| }), | ||
| ) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { useEffect } from 'react' | ||
| import { useTableVisibility } from '../../../hooks' | ||
|
|
||
| export const useSubscribeTableVisibility = () => { | ||
| const { showAllNodes, hideAllNodes } = useTableVisibility() | ||
|
|
||
| // Show all nodes when ⇧A is pressed | ||
| useEffect(() => { | ||
| const down = async (event: KeyboardEvent) => { | ||
| if (event.code === 'KeyA' && event.shiftKey) { | ||
| showAllNodes() | ||
| } | ||
| } | ||
|
|
||
| document.addEventListener('keydown', down) | ||
| return () => document.removeEventListener('keydown', down) | ||
| }, [showAllNodes]) | ||
|
|
||
| // Hide all nodes when ⇧H is pressed | ||
| useEffect(() => { | ||
| const down = async (event: KeyboardEvent) => { | ||
| if (event.code === 'KeyH' && event.shiftKey) { | ||
| hideAllNodes() | ||
| } | ||
| } | ||
|
|
||
| document.addEventListener('keydown', down) | ||
| return () => document.removeEventListener('keydown', down) | ||
| }, [hideAllNodes]) | ||
| } | ||
|
Comment on lines
+1
to
+30
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard global hotkeys against editable targets and simplify handlers The global Consider:
Based on learnings, direct 🤖 Prompt for AI Agents |
||
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.
I noticed that
Show AllandShow All Fieldswere confusing. In the Left Pane, there was aShow Allbutton within the section listing tables, so it seemed natural to omit the word "Tables".I've thought about it and would like to request that you change the labels to
Show All TablesandHide All Tables.Uh oh!
There was an error while loading. Please reload this page.
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.
You're right, thank you for pointing that out. I fixed the literals in a435dd6