Skip to content
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

Use [hidden] on group instead of changing render structure #27

Merged
merged 3 commits into from
Aug 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ Item that becomes active on pointer enter. You should provide a unique `value` f
</Command.Item>
```

### Group `[cmdk-group]`
### Group `[cmdk-group]` `[hidden?]`

Groups items together with the given `heading` (`[cmdk-group-heading]`).

Expand All @@ -202,6 +202,8 @@ Groups items together with the given `heading` (`[cmdk-group-heading]`).
</Command.Group>
```

Groups will not unmount from the DOM, rather the `hidden` attribute is applied to hide it from view. This may be relevant in your styling.

### Separator `[cmdk-separator]`

Visible when the search query is empty or `alwaysRender` is true, hidden otherwise.
Expand Down
12 changes: 7 additions & 5 deletions cmdk/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -627,12 +627,14 @@ const Group = React.forwardRef<HTMLDivElement, GroupProps>((props, forwardedRef)

const inner = <GroupContext.Provider value={id}>{children}</GroupContext.Provider>

// When this group is not rendered, still render rhe children to keep them in the React tree
// however each of them should be rendering `null`, so this won't result in any DOM output
if (!render) return inner

return (
<div ref={mergeRefs([ref, forwardedRef])} {...etc} cmdk-group="" role="presentation">
<div
ref={mergeRefs([ref, forwardedRef])}
{...etc}
cmdk-group=""
role="presentation"
hidden={render ? undefined : true}
>
{heading && (
<div ref={headingRef} cmdk-group-heading="" aria-hidden id={headingId}>
{heading}
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"private": true,
"scripts": {
"build": "pnpm -F cmdk build",
"dev": "pnpm -F cmdk build --watch",
"website": "pnpm -F cmdk-website dev",
"testsite": "pnpm -F cmdk-tests dev",
"test": "playwright test"
Expand Down
2 changes: 1 addition & 1 deletion playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const config: PlaywrightTestConfig = {
},
timeout: 5000,
webServer: {
command: 'npm run test',
command: 'npm run dev',
url: 'http://localhost:3000',
cwd: './test',
reuseExistingServer: !process.env.CI,
Expand Down
12 changes: 6 additions & 6 deletions test/group.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ test.describe('group', async () => {

test('groups are shown/hidden based on item matches', async ({ page }) => {
await page.locator(`[cmdk-input]`).type('z')
await expect(page.locator(`[cmdk-group][data-value="animals"]`)).not.toHaveCount(1)
await expect(page.locator(`[cmdk-group][data-value="letters"]`)).toHaveCount(1)
await expect(page.locator(`[cmdk-group][data-value="animals"]`)).not.toBeVisible()
await expect(page.locator(`[cmdk-group][data-value="letters"]`)).toBeVisible()
})

test('group can be progressively rendered', async ({ page }) => {
await expect(page.locator(`[cmdk-group][data-value="numbers"]`)).not.toHaveCount(1)
await expect(page.locator(`[cmdk-group][data-value="numbers"]`)).not.toBeVisible()
await page.locator(`[cmdk-input]`).type('t')
await expect(page.locator(`[cmdk-group][data-value="animals"]`)).not.toHaveCount(1)
await expect(page.locator(`[cmdk-group][data-value="letters"]`)).not.toHaveCount(1)
await expect(page.locator(`[cmdk-group][data-value="numbers"]`)).toHaveCount(1)
await expect(page.locator(`[cmdk-group][data-value="animals"]`)).not.toBeVisible()
await expect(page.locator(`[cmdk-group][data-value="letters"]`)).not.toBeVisible()
await expect(page.locator(`[cmdk-group][data-value="numbers"]`)).toBeVisible()
})
})
2 changes: 1 addition & 1 deletion test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "cmdk-tests",
"version": "0.0.0",
"scripts": {
"test": "next"
"dev": "next"
},
"dependencies": {
"@types/node": "18.0.4",
Expand Down
2 changes: 1 addition & 1 deletion website/styles/cmdk/raycast.scss
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@
margin: 4px 0;
}

* + [cmdk-group] {
*:not([hidden]) + [cmdk-group] {
margin-top: 8px;
}

Expand Down
6 changes: 3 additions & 3 deletions website/styles/cmdk/vercel.scss
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
background: var(--gray4);
}

&+[cmdk-item] {
& + [cmdk-item] {
margin-top: 4px;
}

Expand Down Expand Up @@ -128,7 +128,7 @@
margin: 4px 0;
}

*+[cmdk-group] {
*:not([hidden]) + [cmdk-group] {
margin-top: 8px;
}

Expand All @@ -151,4 +151,4 @@
white-space: pre-wrap;
color: var(--gray11);
}
}
}