Skip to content
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
13 changes: 0 additions & 13 deletions .cursor/notepads/add-play-tests.md

This file was deleted.

48 changes: 48 additions & 0 deletions .cursor/rules/browser-tests.mdc
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
description: Vitest browser test patterns for design system components
globs: **/__tests__/*.browser.test.tsx
---

# Vitest Browser Tests

Behavioral coverage for design-system components lives next to the component under `__tests__/`, e.g. `ds-checkbox/__tests__/ds-checkbox.browser.test.tsx`. Storybook stories are for docs and controls only.

## Principles

1. **Stories document UI**; assert behavior in `*.browser.test.tsx`.
2. **Spy callbacks with `vi.fn()`** in tests (not Storybook `fn()`).
3. **Prefer a11y queries**: `page.getByRole`, `getByLabelText`, `getByText` -- avoid `getByTestId` unless unavoidable.
4. **Use Vitest browser matchers**: `await expect.element(locator).toBeChecked()`, `toBeDisabled()`, `toBeVisible()`, etc.
5. **Await async UI**: `await locator.click()`, `await expect.element(...)`.
6. **Test user-visible behavior**, not implementation details; use `data-*` only when it is an explicit contract (e.g. indeterminate state).
7. **Disabled / blocked interaction**: `await checkbox.click({ force: true })` when asserting a disabled control does not change state.
8. **Controlled component testing**: define an inline wrapper with `useState` to test controlled behavior.
9. **Rerender for prop changes**: use `const { rerender } = await page.render(...)` to test conditional rendering.
10. **Hover interactions**: `await locator.hover()` / `await locator.unhover()` for tooltip-style components.
11. **Nested locators**: scope queries within a parent: `parentLocator.getByText(...)`.
12. **`toBeInTheDocument` vs `toBeVisible`**: use `not.toBeInTheDocument()` for removed DOM elements, `not.toBeVisible()` for hidden ones.
13. **Querying disabled elements**: pass `{ disabled: true }` to role queries: `page.getByRole('checkbox', { disabled: true })`.

## Example: callback spy

```tsx
import { describe, expect, it, vi } from 'vitest';
import { page } from 'vitest/browser';
import DsWidget from '../ds-widget';

describe('DsWidget', () => {
it('calls onSave when submitted', async () => {
const onSave = vi.fn();
await page.render(<DsWidget onSave={onSave} />);

await page.getByRole('button', { name: /save/i }).click();
expect(onSave).toHaveBeenCalledOnce();
});
});
```

## Run

```bash
pnpm --filter @drivenets/design-system test packages/design-system/src/components/ds-{name}/__tests__/ds-{name}.browser.test.tsx --run
```
68 changes: 47 additions & 21 deletions .cursor/rules/code-review.mdc
Original file line number Diff line number Diff line change
@@ -1,19 +1,40 @@
---
description: Apply when reviewing code changes, running git diff, or preparing a PR
description: Apply when preparing a PR, reviewing code changes, running git diff, or generating a changeset
alwaysApply: false
---

# Code Review Rules
# PR Workflow & Code Review

It is a local code review process done before submitting a PR.
## Git & PR Workflow

## Process
| Requirement | Details |
| --- | --- |
| **No force-push after review** | GitHub loses review history; use merge commits instead |
| **Changelog required** | Run `pnpm changelog` before merge |
| **Conventional commits** | PR title must follow conventional commit format |
| **Separate concerns** | Unrelated changes go in separate PRs; easier revert and isolation |
| **Changeset messages are user-facing** | Write "Add X to Y" or "Fix X in Y", not implementation details |
| **`skip changelog` label** | Use only for non-user-facing changes (CI, tests, docs); never for bug fixes |
| **Use `--frozen-lockfile`** | Run `pnpm install --frozen-lockfile` to avoid unnecessary lockfile changes |
| **Deprecation rules** | Add to `@drivenets/eslint-plugin-design-system` when deprecating |

```markdown
<!-- Good changeset message -->
Add DsCard component

<!-- Bad changeset message - implementation details -->
Refactor card to use CSS modules and fix hover state selector specificity
```

---

## Code Review Process

1. Get diff: `git diff origin/main`
2. Review every changed file against project cursor rules
2. Review every changed file against project cursor rules (scss, react-patterns, storybook, etc.)
3. Flag only clear, high-severity issues (max 10 inline comments)

## Inline comment format
### Inline comment format

```typescript
/**
Expand All @@ -23,19 +44,24 @@ It is a local code review process done before submitting a PR.

- One issue per comment; place on the exact changed line
- Natural tone, specific and actionable; do not mention automated or high-confidence
- Severity emojis: 🚨 Critical 🔒 Security ⚡ Performance ⚠️ Logic ✨ Improvement

## Priorities to check

- No `!important` in SCSS
- No hardcoded colors or spacing (use `--color-*`, `--spacing-*` tokens)
- No `:global` in CSS modules
- No unnecessary `useMemo`/`useCallback`
- No `forwardRef` (deprecated)
- No cross-component internal imports
- No `useLayoutEffect` without DOM reads
- Stories have `play` functions with `fn()` and a11y queries
- No inline styles in stories
- Props layer doesn't expose library internals
- Logic errors, security issues, race conditions, missing edge cases
- Severity: 🚨 Critical 🔒 Security ⚡ Performance ⚠️ Logic ✨ Improvement

---

## PR Checklist

Before submitting a PR:

- [ ] Changelog added (`pnpm changelog`) with user-facing message
- [ ] Behavioral coverage in `*.browser.test.tsx` where interactions matter
- [ ] Browser tests use a11y queries (no test-ids unless unavoidable)
- [ ] CSS uses design tokens, no `!important`
- [ ] No inline styles in stories
- [ ] Props layer doesn't expose library internals
- [ ] Types exported from `.types.ts` with variant arrays
- [ ] Code is well-spaced and readable
- [ ] Matches Figma design
- [ ] Storybook examples show all states (controlled + localized)
- [ ] No cross-component internal imports
- [ ] No unnecessary `useMemo`/`useCallback`
- [ ] Check Ark UI for existing primitives before custom implementation
49 changes: 2 additions & 47 deletions .cursor/rules/design-system.mdc
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
---
description: Component scaffolding templates for creating new design system components
description: Design system component conventions and primitive library choices
globs: packages/design-system/src/components/**/*
---

# Component Scaffolding
# Design System Components

## File Structure

Expand All @@ -18,51 +18,6 @@ ds-{name}/
└── ds-{name}.stories.tsx # Stories
```

## Implementation Template

```tsx
import classNames from 'classnames';
import styles from './ds-{name}.module.scss';
import type { Ds{Name}Props } from './ds-{name}.types';

/**
* Design system {Name} component
*/
const Ds{Name} = ({
className,
children,
...props
}: Ds{Name}Props) => {
return (
<div className={classNames(styles.container, className)} {...props}>
{children}
</div>
);
};

export default Ds{Name};
```

## Types Template

```typescript
import type { ComponentPropsWithoutRef, ReactNode } from 'react';

export interface Ds{Name}Props extends ComponentPropsWithoutRef<'div'> {
/**
* JSDoc description
*/
label?: ReactNode;
}
```

## Index Barrel

```typescript
export { default as Ds{Name} } from './ds-{name}';
export type { Ds{Name}Props } from './ds-{name}.types';
```

## Primitive Libraries

1. **Ark UI** (`@ark-ui/react`) - Primary choice for new components
Expand Down
42 changes: 1 addition & 41 deletions .cursor/rules/react-patterns.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,7 @@ const id = useId();
container.addEventListener('scroll', checkOverflow, { passive: true });
```

One other situation you might want to use useLayoutEffect instead of useEffect is if you're updating a value (like a ref)
and you want to make sure it's up-to-date before any other code runs. For example:

```tsx
// Good
const ref = React.useRef()
React.useEffect(() => {
ref.current = 'some value'
})

// then, later in another hook or something
React.useLayoutEffect(() => {
console.log(ref.current) // <-- this logs an old value because this runs first!
})
```
Also use `useLayoutEffect` when updating a ref that must be current before other hooks read it (it fires before `useEffect`).

---

Expand Down Expand Up @@ -144,29 +130,3 @@ const [inputValue, setInputValue] = useState('');
</Combobox.Root>
```

---

## Component API Ordering

| Requirement | Details |
| -------------------------------------------- | -------------------------------------------------------------------------------- |
| **Value props first, callbacks last** | Order: value/config props, then render/slot props, then callbacks |
| **`locale` prop for i18n** | Components with hardcoded text should accept a `locale` prop for overrides |

```typescript
// Good - ordered interface
export interface DsTagFilterProps {
// Value props
items: TagFilterItem[];
selectedKeys?: string[];
size?: 'small' | 'medium';

// Slot props
slotProps?: { tag?: Partial<DsTagProps> };
locale?: { clearAll?: string; showMore?: string };

// Callbacks
onSelectionChange?: (keys: string[]) => void;
onClear?: () => void;
}
```
59 changes: 4 additions & 55 deletions .cursor/rules/standards.mdc
Original file line number Diff line number Diff line change
@@ -1,35 +1,9 @@
---
description: PR requirements and code standards for the design-system repository
description: Code standards for the design-system repository
alwaysApply: true
---

# Pull Request Requirements & Code Standards

Based on code review feedback, here are the consolidated standards for the design-system repository.

---

## Git & PR Workflow

| Requirement | Details |
| -------------------------------------------- | -------------------------------------------------------------------------------- |
| **No force-push after review** | GitHub loses review history; use merge commits instead |
| **Changelog required** | Run `pnpm changelog` before merge |
| **Conventional commits** | PR title must follow conventional commit format |
| **Separate concerns** | Unrelated changes go in separate PRs; easier revert and isolation |
| **Changeset messages are user-facing** | Write "Add X to Y" or "Fix X in Y", not implementation details |
| **`skip changelog` label** | Use only for non-user-facing changes (CI, tests, docs); never for bug fixes |
| **Use `--frozen-lockfile`** | Run `pnpm install --frozen-lockfile` to avoid unnecessary lockfile changes |

```markdown
<!-- Good changeset message -->
Add DsCard component

<!-- Bad changeset message - implementation details -->
Refactor card to use CSS modules and fix hover state selector specificity
```

---
# Code Standards

## Component API Design

Expand All @@ -42,6 +16,8 @@ Refactor card to use CSS modules and fix hover state selector specificity
| **Consistent callback naming** | Prefer `onXChange` pattern |
| **`null` for empty callback values** | Use `null` (not `undefined`) for optional values in callbacks: `(value: string \| null) => void` |
| **Single deprecation comment** | In types file only, not on every export |
| **Value props first, callbacks last** | Order: value/config props, then render/slot props, then callbacks |
| **`locale` prop for i18n** | Components with hardcoded text should accept a `locale` prop for overrides |

```typescript
// Good - own props layer with null for clearable value
Expand Down Expand Up @@ -145,30 +121,3 @@ import { formatDate } from '../../utils/format-date';
| **Barrel exports** | Use `index.ts` not `index.tsx` |
| **kebab-case files** | All files must be kebab-case |

---

## ESLint Plugin

| Requirement | Details |
| -------------------------------------------- | -------------------------------------------------------------------------------- |
| **Deprecation rules** | Add to `@drivenets/eslint-plugin-design-system` when deprecating |

---

## PR Checklist

Before submitting a PR:

- [ ] Changelog added (`pnpm changelog`) with user-facing message
- [ ] All stories have `play` tests with `fn()` for callbacks
- [ ] Tests use a11y queries (no test-ids)
- [ ] CSS uses design tokens, no `!important`
- [ ] No inline styles in stories
- [ ] Props layer doesn't expose library internals
- [ ] Types exported from `.types.ts` with variant arrays
- [ ] Code is well-spaced and readable
- [ ] Matches Figma design
- [ ] Storybook examples show off all states (including controlled + localized)
- [ ] No cross-component internal imports
- [ ] No unnecessary `useMemo`/`useCallback`
- [ ] Check Ark UI for existing primitives before custom implementation
Loading
Loading