diff --git a/claude.md b/claude.md index ea64a55d..8d092781 100644 --- a/claude.md +++ b/claude.md @@ -2,6 +2,48 @@ - There may be multiple environments running simultaneously using different worktrees. To determine the corren environment, you can get port numbers and env name from the root .env file. - When refactoring module names, run `grep -r "old_module_name" .` before committing to catch all remaining references (especially entry points like `main.py`). Use `__init__.py` re-exports for backward compatibility. +## Frontend Development Workflow + +**BEFORE writing ANY frontend code, follow this workflow:** + +### Step 1: Read Quick Reference +Read `ushadow/frontend/AGENT_QUICK_REF.md` - it's ~800 tokens and covers all reusable components. + +### Step 2: Search for Existing Components +```bash +# Search for components before creating new ones +grep -r "ComponentName" ushadow/frontend/src/components/ + +# Check available hooks +cat ushadow/frontend/src/hooks/index.ts + +# Check available contexts +ls ushadow/frontend/src/contexts/ +``` + +### Step 3: Check UI Contract +Read `ushadow/frontend/src/testing/ui-contract.ts` for: +- Component documentation and examples +- TestID patterns (use these, don't invent new ones) +- Import paths + +### Step 4: Follow Patterns +- **Hooks**: See `ushadow/frontend/src/hooks/HOOK_PATTERNS.md` +- **State**: Use existing contexts, React Query for server state +- **Forms**: Use react-hook-form + Controller pattern + +### File Size Limits (ESLint enforced) +- **Pages**: Max 600 lines → Extract logic to hooks, UI to components +- **Components**: Max 300 lines → Split into smaller components +- **Hooks**: Max 100 lines → Compose from smaller hooks + +### What NOT to Do +- ❌ Create custom modals → Use `Modal` component +- ❌ Create custom secret inputs → Use `SecretInput` +- ❌ Create new state management → Use existing contexts +- ❌ Hardcode testid strings → Import from `ui-contract.ts` +- ❌ Put business logic in components → Extract to hooks + ## CRITICAL Frontend Development Rules **MANDATORY: Every frontend change MUST include `data-testid` attributes for ALL interactive elements.** diff --git a/docs/FRONTEND-EXCELLENCE-PLAN.md b/docs/FRONTEND-EXCELLENCE-PLAN.md new file mode 100644 index 00000000..c2d0c146 --- /dev/null +++ b/docs/FRONTEND-EXCELLENCE-PLAN.md @@ -0,0 +1,580 @@ +# Frontend Excellence Plan + +> A strategy for modular, consistent frontend code that AI agents can reliably reference and reuse. + +## Problem Statement + +AI agents currently: +1. Re-implement features instead of reusing existing components +2. Generate inconsistent code styles across pages +3. Create messy, sprawling code that's expensive to reference (high token usage) +4. Miss existing patterns and conventions + +## Goals + +1. **Component Reuse**: Agents discover and use existing components 90%+ of the time +2. **Consistency**: Generated code follows established patterns automatically +3. **Token Efficiency**: Reference docs fit in ~2K tokens, not 20K +4. **First-Time Quality**: Code passes review without major restructuring + +--- + +## Phase 1: Component Registry (Foundation) + +### 1.1 Create Component Index + +Create a single source of truth that agents can quickly scan: + +**File**: `frontend/src/components/COMPONENT_REGISTRY.md` + +```markdown +# Component Registry + +## Form Components +| Component | Import | Use Case | Example | +|-----------|--------|----------|---------| +| SecretInput | `@/components/settings/SecretInput` | API keys, passwords | `` | +| SettingField | `@/components/settings/SettingField` | Generic fields | `` | + +## Layout Components +| Component | Import | Use Case | +|-----------|--------|----------| +| Modal | `@/components/Modal` | All modals - REQUIRED | +| ConfirmDialog | `@/components/ConfirmDialog` | Destructive actions | +| SettingsSection | `@/components/settings/SettingsSection` | Group related settings | + +## Forbidden Patterns +- ❌ `fixed inset-0` DIVs → Use `Modal` component +- ❌ Custom input with eye icon → Use `SecretInput` +- ❌ Inline modal state → Use `useModal` hook +``` + +### 1.2 Add Path Aliases + +Update `tsconfig.json`: + +```json +{ + "compilerOptions": { + "paths": { + "@/*": ["./src/*"], + "@/components/*": ["./src/components/*"], + "@/hooks/*": ["./src/hooks/*"], + "@/contexts/*": ["./src/contexts/*"] + } + } +} +``` + +**Why**: Agents produce cleaner imports, easier to grep/find. + +--- + +## Phase 2: ESLint Enforcement + +### 2.1 Install Additional ESLint Plugins + +```bash +npm install -D \ + eslint-plugin-import \ + eslint-plugin-jsx-a11y \ + eslint-plugin-react \ + eslint-plugin-boundaries \ + @tanstack/eslint-plugin-query +``` + +### 2.2 Create Flat Config + +**File**: `frontend/eslint.config.js` + +```javascript +import js from '@eslint/js' +import typescript from '@typescript-eslint/eslint-plugin' +import tsParser from '@typescript-eslint/parser' +import react from 'eslint-plugin-react' +import reactHooks from 'eslint-plugin-react-hooks' +import importPlugin from 'eslint-plugin-import' +import boundaries from 'eslint-plugin-boundaries' +import jsxA11y from 'eslint-plugin-jsx-a11y' + +export default [ + js.configs.recommended, + { + files: ['**/*.{ts,tsx}'], + languageOptions: { + parser: tsParser, + parserOptions: { + project: './tsconfig.json', + }, + }, + plugins: { + '@typescript-eslint': typescript, + 'react': react, + 'react-hooks': reactHooks, + 'import': importPlugin, + 'boundaries': boundaries, + 'jsx-a11y': jsxA11y, + }, + settings: { + 'boundaries/elements': [ + { type: 'components', pattern: 'src/components/*' }, + { type: 'pages', pattern: 'src/pages/*' }, + { type: 'hooks', pattern: 'src/hooks/*' }, + { type: 'contexts', pattern: 'src/contexts/*' }, + { type: 'services', pattern: 'src/services/*' }, + ], + }, + rules: { + // === COMPONENT REUSE ENFORCEMENT === + + // Prevent pages from having 300+ lines (force extraction) + 'max-lines': ['warn', { max: 300, skipComments: true, skipBlankLines: true }], + + // Force components to be small and focused + 'max-lines-per-function': ['warn', { max: 80, skipComments: true }], + + // === IMPORT ORGANIZATION === + 'import/order': ['error', { + 'groups': [ + 'builtin', + 'external', + 'internal', + ['parent', 'sibling'], + 'index' + ], + 'pathGroups': [ + { pattern: 'react', group: 'builtin', position: 'before' }, + { pattern: '@/**', group: 'internal' } + ], + 'newlines-between': 'always', + 'alphabetize': { order: 'asc' } + }], + + // === ARCHITECTURE BOUNDARIES === + 'boundaries/element-types': ['error', { + default: 'disallow', + rules: [ + // Pages can import components, hooks, contexts, services + { from: 'pages', allow: ['components', 'hooks', 'contexts', 'services'] }, + // Components can import other components and hooks + { from: 'components', allow: ['components', 'hooks'] }, + // Hooks can import other hooks and services + { from: 'hooks', allow: ['hooks', 'services', 'contexts'] }, + ] + }], + + // === REACT BEST PRACTICES === + 'react-hooks/rules-of-hooks': 'error', + 'react-hooks/exhaustive-deps': 'warn', + 'react/jsx-no-duplicate-props': 'error', + 'react/jsx-key': 'error', + + // === ACCESSIBILITY === + 'jsx-a11y/alt-text': 'error', + 'jsx-a11y/anchor-has-content': 'error', + 'jsx-a11y/click-events-have-key-events': 'warn', + + // === PREVENT COMMON AGENT MISTAKES === + 'no-console': ['warn', { allow: ['warn', 'error'] }], + '@typescript-eslint/no-explicit-any': 'warn', + '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], + }, + }, +] +``` + +### 2.3 Add Custom Rule for Forbidden Patterns + +Create `frontend/eslint-rules/no-inline-modals.js`: + +```javascript +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'Disallow inline modal implementations - use Modal component', + }, + messages: { + useModalComponent: 'Use from @/components/Modal instead of inline fixed positioning', + }, + }, + create(context) { + return { + JSXAttribute(node) { + if ( + node.name.name === 'className' && + node.value?.value?.includes('fixed inset-0') + ) { + context.report({ node, messageId: 'useModalComponent' }) + } + }, + } + }, +} +``` + +--- + +## Phase 3: Hook & Context Patterns + +### 3.1 Create Standard Hook Templates + +**File**: `frontend/src/hooks/HOOK_PATTERNS.md` + +```markdown +# Hook Patterns + +## Data Fetching Hook Pattern +Use for any API data with caching: + +\`\`\`typescript +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' + +export function useResource(id: string) { + const queryClient = useQueryClient() + + const query = useQuery({ + queryKey: ['resource', id], + queryFn: () => api.getResource(id), + staleTime: 30_000, + }) + + const mutation = useMutation({ + mutationFn: api.updateResource, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['resource', id] }) + }, + }) + + return { ...query, update: mutation.mutate } +} +\`\`\` + +## Form State Hook Pattern +Use for forms with validation: + +\`\`\`typescript +import { useForm } from 'react-hook-form' +import { zodResolver } from '@hookform/resolvers/zod' + +const schema = z.object({ ... }) + +export function useMyForm(defaults: FormData) { + return useForm({ + resolver: zodResolver(schema), + defaultValues: defaults, + }) +} +\`\`\` +``` + +### 3.2 Add Missing Utility Hooks + +Create commonly needed hooks that agents keep re-implementing: + +```typescript +// frontend/src/hooks/useModal.ts +export function useModal(initialOpen = false) { + const [isOpen, setIsOpen] = useState(initialOpen) + return { + isOpen, + open: () => setIsOpen(true), + close: () => setIsOpen(false), + toggle: () => setIsOpen(v => !v), + } +} + +// frontend/src/hooks/useClipboard.ts +export function useClipboard(timeout = 2000) { + const [copied, setCopied] = useState(false) + + const copy = async (text: string) => { + await navigator.clipboard.writeText(text) + setCopied(true) + setTimeout(() => setCopied(false), timeout) + } + + return { copied, copy } +} + +// frontend/src/hooks/useDebounce.ts +export function useDebounce(value: T, delay: number): T { + const [debouncedValue, setDebouncedValue] = useState(value) + + useEffect(() => { + const timer = setTimeout(() => setDebouncedValue(value), delay) + return () => clearTimeout(timer) + }, [value, delay]) + + return debouncedValue +} +``` + +--- + +## Phase 4: Agent-Readable Documentation + +### 4.1 Create Compact Reference Files + +The key insight: **Agents need scannable, compact references—not verbose docs.** + +**File**: `frontend/AGENT_QUICK_REF.md` (~1500 tokens) + +```markdown +# Frontend Quick Reference for AI Agents + +## Before You Code +1. Check COMPONENT_REGISTRY.md for existing components +2. Check hooks/index.ts for existing hooks +3. Use @/ path aliases + +## Component Patterns + +### Modal (ALWAYS use this) +\`\`\`tsx +import Modal from '@/components/Modal' + + {content} + +\`\`\` + +### Form with Validation +\`\`\`tsx +import { useForm, Controller } from 'react-hook-form' +import { SecretInput } from '@/components/settings/SecretInput' + +const { control, handleSubmit } = useForm() + + ( + +)} /> +\`\`\` + +### Data Fetching +\`\`\`tsx +import { useQuery } from '@tanstack/react-query' +const { data, isLoading } = useQuery({ queryKey: ['x'], queryFn: fetchX }) +\`\`\` + +## Test IDs (REQUIRED) +- All interactive elements need `data-testid` +- Pattern: `{context}-{element}` e.g., `settings-save-btn` + +## Styling +- Use Tailwind classes +- Dark mode: prefix with `dark:` +- Colors: primary-*, accent-*, surface-*, semantic (success/warning/error/info) + +## Don't +- ❌ Create custom modals with `fixed inset-0` +- ❌ Skip data-testid attributes +- ❌ Create new state management (use existing contexts) +- ❌ Inline long className strings (extract to component) +``` + +### 4.2 Add Component JSDoc Examples + +Every reusable component should have a clear JSDoc with example: + +```typescript +/** + * SecretInput - API key/password input with visibility toggle + * + * @example + * // With react-hook-form + * ( + * + * )} + * /> + * + * @example + * // Standalone + * + */ +``` + +--- + +## Phase 5: Pre-Commit Checks + +### 5.1 Add Husky + lint-staged + +```bash +npm install -D husky lint-staged +npx husky init +``` + +**File**: `.husky/pre-commit` + +```bash +#!/bin/sh +npx lint-staged +``` + +**File**: `package.json` (add) + +```json +{ + "lint-staged": { + "src/**/*.{ts,tsx}": [ + "eslint --fix --max-warnings 0", + "prettier --write" + ] + } +} +``` + +### 5.2 Add data-testid Checker + +**File**: `scripts/check-testids.js` + +```javascript +#!/usr/bin/env node +/** + * Verify all interactive elements have data-testid + */ +import { glob } from 'glob' +import fs from 'fs' + +const INTERACTIVE_PATTERNS = [ + /]*(?!data-testid)/gi, + /]*(?!data-testid)/gi, + /]*(?!data-testid)/gi, + /onClick=\{[^}]+\}[^>]*(?!data-testid)/gi, +] + +const files = await glob('src/**/*.tsx') +let errors = [] + +for (const file of files) { + const content = fs.readFileSync(file, 'utf8') + // Check for interactive elements without testid + // ... implementation +} + +if (errors.length > 0) { + console.error('Missing data-testid attributes:') + errors.forEach(e => console.error(` ${e}`)) + process.exit(1) +} +``` + +--- + +## Phase 6: Directory Structure Conventions + +### 6.1 Enforce Flat Component Organization + +``` +src/ +├── components/ +│ ├── COMPONENT_REGISTRY.md # Agent-readable index +│ ├── common/ # Truly shared (Modal, Button, etc.) +│ │ ├── Modal.tsx +│ │ ├── ConfirmDialog.tsx +│ │ └── index.ts +│ ├── forms/ # Form-related components +│ │ ├── SecretInput.tsx +│ │ ├── SettingField.tsx +│ │ └── index.ts +│ ├── layout/ # Layout components +│ │ └── index.ts +│ └── [feature]/ # Feature-specific components +│ └── index.ts # Always export from index +├── hooks/ +│ ├── HOOK_PATTERNS.md # Agent-readable patterns +│ ├── index.ts # Central export +│ ├── useModal.ts +│ └── use[Feature].ts +├── contexts/ +│ └── index.ts # Central export +├── pages/ +│ └── [Page].tsx # Max 300 lines +└── AGENT_QUICK_REF.md # Top-level agent reference +``` + +### 6.2 Index File Convention + +Every directory MUST have an `index.ts` that exports all public APIs: + +```typescript +// components/forms/index.ts +export { SecretInput } from './SecretInput' +export { SettingField } from './SettingField' +export type { SecretInputProps } from './SecretInput' +``` + +--- + +## Phase 7: Agent Instructions (CLAUDE.md Updates) + +### 7.1 Add to CLAUDE.md + +```markdown +## Frontend Development Workflow + +### Before Creating ANY Component +1. **Search first**: `grep -r "ComponentName" src/components/` +2. **Check registry**: Read `src/components/COMPONENT_REGISTRY.md` +3. **Check hooks**: Read `src/hooks/index.ts` + +### When Creating New Components +1. Add to appropriate directory under `src/components/` +2. Export from directory's `index.ts` +3. Add entry to `COMPONENT_REGISTRY.md` +4. Include JSDoc with @example + +### Required Patterns +- All modals: Use `` from `@/components/common/Modal` +- All forms: Use react-hook-form + Controller pattern +- All API calls: Use @tanstack/react-query hooks +- All interactive elements: Include `data-testid` + +### File Size Limits +- Pages: Max 300 lines (extract to components) +- Components: Max 150 lines (split if larger) +- Hooks: Max 100 lines (compose smaller hooks) +``` + +--- + +## Implementation Priority + +| Phase | Effort | Impact | Do First? | +|-------|--------|--------|-----------| +| 1. Component Registry | Low | High | ✅ Yes | +| 2. ESLint Rules | Medium | High | ✅ Yes | +| 4. Agent Quick Ref | Low | High | ✅ Yes | +| 3. Hook Patterns | Low | Medium | Week 2 | +| 5. Pre-Commit | Medium | Medium | Week 2 | +| 6. Directory Cleanup | Medium | Medium | Week 3 | +| 7. CLAUDE.md Updates | Low | High | ✅ Yes | + +--- + +## Measuring Success + +### Before Metrics +- How often do agents re-implement existing components? +- Average lines per page file? +- ESLint warnings per PR? + +### After Metrics (Targets) +- Component reuse: 90%+ of PRs use existing components +- Page file size: 95% under 300 lines +- ESLint: 0 warnings on all PRs +- First-pass review: 80%+ PRs pass without "extract this" comments + +--- + +## Quick Wins (Do Today) + +1. **Create `AGENT_QUICK_REF.md`** - Takes 30 min, immediate impact +2. **Create `COMPONENT_REGISTRY.md`** - Takes 1 hour, agents use it immediately +3. **Add `max-lines` ESLint rule** - 5 min config change, forces extraction +4. **Update CLAUDE.md** - Add "search before creating" instructions + +These four changes will immediately improve agent output quality without major refactoring. diff --git a/ushadow/frontend/AGENT_QUICK_REF.md b/ushadow/frontend/AGENT_QUICK_REF.md new file mode 100644 index 00000000..07f466f6 --- /dev/null +++ b/ushadow/frontend/AGENT_QUICK_REF.md @@ -0,0 +1,222 @@ +# Frontend Quick Reference + +> Read this BEFORE writing any frontend code. ~800 tokens. + +## Workflow + +1. **Search first** - `grep -r "ComponentName" src/components/` +2. **Check hooks** - Read `src/hooks/index.ts` +3. **Check contexts** - Read `src/contexts/` for global state +4. **Use existing patterns** - Copy from similar pages + +## Reusable Components + +### Modal (REQUIRED for all dialogs) +```tsx +import Modal from '@/components/Modal' + + + {content} + +``` +❌ **NEVER** use `fixed inset-0` divs - always use Modal component + +### SecretInput (API keys, passwords) +```tsx +import { SecretInput } from '@/components/settings/SecretInput' + + +``` + +### SettingField (generic form field) +```tsx +import { SettingField } from '@/components/settings/SettingField' + + +// Types: 'text' | 'secret' | 'url' | 'select' | 'toggle' +``` + +### ConfirmDialog (destructive actions) +```tsx +import ConfirmDialog from '@/components/ConfirmDialog' + + +``` + +### SettingsSection (group related settings) +```tsx +import { SettingsSection } from '@/components/settings/SettingsSection' + + + + +``` + +## Hooks + +| Hook | Use Case | +|------|----------| +| `useModal()` | Modal open/close state | +| `useServiceStatus()` | Service health polling | +| `useServiceStart()` | Start service with port conflict handling | +| `useMemories()` | Memory CRUD operations | +| `useQrCode()` | Generate QR codes | + +## State Management + +- **Server state**: Use `@tanstack/react-query` +- **Form state**: Use `react-hook-form` + `zod` +- **Global state**: Use existing contexts (Auth, Theme, Services, Wizard) +- **Local UI state**: Use `useState` + +### React Query pattern +```tsx +const { data, isLoading } = useQuery({ + queryKey: ['resource', id], + queryFn: () => api.getResource(id), +}) +``` + +### Form pattern +```tsx +import { useForm, Controller } from 'react-hook-form' + +const { control, handleSubmit } = useForm({ defaultValues }) + + ( + +)} /> +``` + +## Styling + +- Use Tailwind classes +- Dark mode: `dark:` prefix +- Colors: `primary-*`, `accent-*`, `surface-*` +- Semantic: `text-success-*`, `text-warning-*`, `text-error-*` + +## Required: data-testid + +ALL interactive elements need `data-testid`: +```tsx + + +``` + +See `src/testing/ui-contract.ts` for patterns. + +## File Size Limits + +- Pages: **max 600 lines** (extract components if larger) +- Components: **max 300 lines** (split if larger) +- Hooks: **max 100 lines** (compose smaller hooks) + +## Common UI Bugs (AVOID THESE) + +### 1. Z-Index / Stacking Issues +```tsx +// ❌ BAD - arbitrary z-index values in page components +className="z-[999]" +className="z-50" + +// ✅ GOOD - use defined scale for non-portaled elements +className="z-sticky" // 40 - sticky headers +className="z-dropdown" // 50 - inline menus, dropdowns +className="z-modal" // 60 - overlays (if not using Modal) +className="z-toast" // 70 - notifications +``` +**Rule**: Always use `Modal` component for dialogs (it portals to body). For inline dropdowns, use `z-dropdown`. + +### 2. Menus/Dropdowns Getting Cutoff +```tsx +// ❌ BAD - dropdown trapped in overflow:hidden parent +
+ // Gets clipped! +
+ +// ✅ GOOD - use overflow-visible or portal for dropdowns +
+ +
+// Or render dropdown in a portal +``` + +### 3. Text Not Truncating +```tsx +// ❌ BAD - text overflows container +{longText} + +// ✅ GOOD - truncate with ellipsis +{longText} + +// ✅ GOOD - multi-line truncate (2 lines) +

{longText}

+``` +**Note**: `truncate` requires a width constraint (parent with `w-*` or `flex` with `min-w-0`) + +### 4. Layout Shift When Content Expands +```tsx +// ❌ BAD - drawer/panel pushes siblings +
+ // When this expands... + // ...this shifts! +
+ +// ✅ GOOD - use fixed dimensions or absolute positioning +
+ +
// Takes remaining space + +
+
+ +// ✅ GOOD - overlay doesn't affect layout +
+ + // Overlays, doesn't push +
+``` + +### 5. State Not Reflecting Status (Start/Stop bugs) +```tsx +// ❌ BAD - optimistic update without error handling +const handleStart = () => { + setStatus('running') // Assumes success! + api.startService() +} + +// ✅ GOOD - update state AFTER confirmation +const handleStart = async () => { + setIsStarting(true) + try { + await api.startService() + // Don't manually set status - let the query refetch + await queryClient.invalidateQueries(['service', id]) + } finally { + setIsStarting(false) + } +} + +// ✅ BEST - use the useServiceStatus hook +const status = useServiceStatus(service, config, containerStatus) +// Status derived from actual data, not local state +``` + +## Don't + +- ❌ Create custom modals - use `Modal` +- ❌ Create custom secret inputs - use `SecretInput` +- ❌ Skip data-testid attributes +- ❌ Create new contexts without checking existing ones +- ❌ Inline 20+ line className strings - extract component +- ❌ Use arbitrary z-index values - use `z-dropdown`, `z-modal`, `z-toast` +- ❌ Optimistically update status - fetch actual state after actions diff --git a/ushadow/frontend/e2e/pom/BasePage.ts b/ushadow/frontend/e2e/pom/BasePage.ts index 15bb7e06..06e32207 100644 --- a/ushadow/frontend/e2e/pom/BasePage.ts +++ b/ushadow/frontend/e2e/pom/BasePage.ts @@ -2,9 +2,17 @@ * BasePage - Base class for all Page Object Models. * * Provides common navigation and utility methods. + * Uses testid patterns from ui-contract.ts for consistency. */ import { type Page, type Locator } from '@playwright/test' +import { + secretInput, + settingField, + settingsSection, + envVarEditor, + modal, +} from '../../src/testing/ui-contract' export abstract class BasePage { readonly page: Page @@ -30,64 +38,92 @@ export abstract class BasePage { return this.page.getByTestId(testId) } - /** - * Get a locator for a setting field by ID - */ + // =========================================================================== + // Setting Field helpers (uses ui-contract patterns) + // =========================================================================== + protected getSettingField(id: string): Locator { - return this.getByTestId(`setting-field-${id}`) + return this.getByTestId(settingField.container(id)) } - /** - * Get a locator for a secret input by ID - */ - protected getSecretInput(id: string): Locator { - return this.getByTestId(`secret-input-${id}`) + async fillSetting(id: string, value: string): Promise { + const input = this.getByTestId(settingField.input(id)) + await input.fill(value) } - /** - * Get a locator for a settings section by ID - */ - protected getSettingsSection(id: string): Locator { - return this.getByTestId(`settings-section-${id}`) + async selectSetting(id: string, value: string): Promise { + const select = this.getByTestId(settingField.select(id)) + await select.selectOption(value) + } + + async toggleSetting(id: string): Promise { + const toggle = this.getByTestId(settingField.toggle(id)) + await toggle.click() + } + + // =========================================================================== + // Secret Input helpers (uses ui-contract patterns) + // =========================================================================== + + protected getSecretInput(id: string): Locator { + return this.getByTestId(secretInput.container(id)) } - /** - * Fill a secret input field - */ async fillSecret(id: string, value: string): Promise { - const field = this.getByTestId(`secret-input-${id}-field`) + const field = this.getByTestId(secretInput.field(id)) await field.fill(value) } - /** - * Fill a text/url setting field - */ - async fillSetting(id: string, value: string): Promise { - const input = this.getByTestId(`setting-field-${id}-input`) + async toggleSecretVisibility(id: string): Promise { + const toggle = this.getByTestId(secretInput.toggle(id)) + await toggle.click() + } + + // =========================================================================== + // Settings Section helpers (uses ui-contract patterns) + // =========================================================================== + + protected getSettingsSection(id: string): Locator { + return this.getByTestId(settingsSection.container(id)) + } + + // =========================================================================== + // Env Var Editor helpers (uses ui-contract patterns) + // =========================================================================== + + protected getEnvVarEditor(varName: string): Locator { + return this.getByTestId(envVarEditor.container(varName)) + } + + async fillEnvVarValue(varName: string, value: string): Promise { + const input = this.getByTestId(envVarEditor.valueInput(varName)) await input.fill(value) } - /** - * Select an option in a setting field - */ - async selectSetting(id: string, value: string): Promise { - const select = this.getByTestId(`setting-field-${id}-select`) - await select.selectOption(value) + async selectEnvVarMapping(varName: string, settingPath: string): Promise { + // First click the Map button to show the dropdown + const mapBtn = this.getByTestId(envVarEditor.mapButton(varName)) + await mapBtn.click() + // Then select the setting + const select = this.getByTestId(envVarEditor.mapSelect(varName)) + await select.selectOption(settingPath) } - /** - * Toggle a setting switch - */ - async toggleSetting(id: string): Promise { - const toggle = this.getByTestId(`setting-field-${id}-toggle`) - await toggle.click() + // =========================================================================== + // Modal helpers (uses ui-contract patterns) + // =========================================================================== + + protected getModal(id: string): Locator { + return this.getByTestId(modal.container(id)) } - /** - * Toggle secret visibility - */ - async toggleSecretVisibility(id: string): Promise { - const toggle = this.getByTestId(`secret-input-${id}-toggle`) - await toggle.click() + async closeModal(id: string): Promise { + const closeBtn = this.getByTestId(modal.close(id)) + await closeBtn.click() + } + + async clickModalBackdrop(id: string): Promise { + const backdrop = this.getByTestId(modal.backdrop(id)) + await backdrop.click() } } diff --git a/ushadow/frontend/eslint.config.js b/ushadow/frontend/eslint.config.js new file mode 100644 index 00000000..87ab9c50 --- /dev/null +++ b/ushadow/frontend/eslint.config.js @@ -0,0 +1,109 @@ +import js from '@eslint/js' +import tsPlugin from '@typescript-eslint/eslint-plugin' +import tsParser from '@typescript-eslint/parser' +import reactHooks from 'eslint-plugin-react-hooks' +import reactRefresh from 'eslint-plugin-react-refresh' + +export default [ + js.configs.recommended, + { + ignores: ['dist/**', 'node_modules/**'], + }, + { + files: ['**/*.{ts,tsx}'], + languageOptions: { + parser: tsParser, + parserOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + ecmaFeatures: { jsx: true }, + }, + globals: { + // Browser globals + window: 'readonly', + document: 'readonly', + navigator: 'readonly', + console: 'readonly', + setTimeout: 'readonly', + clearTimeout: 'readonly', + setInterval: 'readonly', + clearInterval: 'readonly', + fetch: 'readonly', + localStorage: 'readonly', + sessionStorage: 'readonly', + HTMLElement: 'readonly', + HTMLInputElement: 'readonly', + HTMLButtonElement: 'readonly', + KeyboardEvent: 'readonly', + MouseEvent: 'readonly', + Event: 'readonly', + EventTarget: 'readonly', + FormData: 'readonly', + URL: 'readonly', + URLSearchParams: 'readonly', + AbortController: 'readonly', + RequestInit: 'readonly', + Response: 'readonly', + Blob: 'readonly', + File: 'readonly', + FileReader: 'readonly', + WebSocket: 'readonly', + MutationObserver: 'readonly', + ResizeObserver: 'readonly', + IntersectionObserver: 'readonly', + requestAnimationFrame: 'readonly', + cancelAnimationFrame: 'readonly', + crypto: 'readonly', + performance: 'readonly', + alert: 'readonly', + confirm: 'readonly', + prompt: 'readonly', + }, + }, + plugins: { + '@typescript-eslint': tsPlugin, + 'react-hooks': reactHooks, + 'react-refresh': reactRefresh, + }, + rules: { + // TypeScript rules + ...tsPlugin.configs.recommended.rules, + '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], + '@typescript-eslint/no-explicit-any': 'warn', + + // React hooks rules + ...reactHooks.configs.recommended.rules, + + // React refresh + 'react-refresh/only-export-components': ['warn', { allowConstantExport: true }], + + // ========================================================================= + // CODE QUALITY - Prevents sprawling files, forces modular code + // ========================================================================= + + // Pages should be max 600 lines - forces extraction to components/hooks + 'max-lines': ['warn', { + max: 600, + skipBlankLines: true, + skipComments: true, + }], + + // Functions should be focused - max 80 lines + 'max-lines-per-function': ['warn', { + max: 80, + skipBlankLines: true, + skipComments: true, + IIFEs: true, + }], + + // Prevent deeply nested code - max 4 levels + 'max-depth': ['warn', 4], + + // Limit complexity - forces simpler logic + 'complexity': ['warn', 15], + + // Prevent console.log in production (warn only) + 'no-console': ['warn', { allow: ['warn', 'error'] }], + }, + }, +] diff --git a/ushadow/frontend/src/components/settings/SecretInput.tsx b/ushadow/frontend/src/components/settings/SecretInput.tsx index ad3d3b10..9be56044 100644 --- a/ushadow/frontend/src/components/settings/SecretInput.tsx +++ b/ushadow/frontend/src/components/settings/SecretInput.tsx @@ -1,15 +1,14 @@ /** * SecretInput - Reusable secret/API key input with visibility toggle. * - * Features: - * - Show/hide toggle for secret values - * - Masked display for sensitive data - * - Consistent test IDs for Playwright automation + * @see src/testing/ui-contract.ts for testid patterns and usage examples */ import { useState, forwardRef } from 'react' import { Eye, EyeOff, Key } from 'lucide-react' +import { secretInput } from '../../testing/ui-contract' + export interface SecretInputProps { /** Unique identifier for the input (used in test IDs) */ id: string @@ -48,10 +47,8 @@ export const SecretInput = forwardRef( ) => { const [visible, setVisible] = useState(false) - const testId = `secret-input-${id}` - return ( -
+
{showIcon && ( @@ -65,7 +62,7 @@ export const SecretInput = forwardRef( onChange={(e) => onChange(e.target.value)} placeholder={placeholder} disabled={disabled} - data-testid={`${testId}-field`} + data-testid={secretInput.field(id)} className={` w-full rounded-lg border px-3 py-2 pr-10 ${showIcon ? 'pl-10' : 'pl-3'} @@ -85,7 +82,7 @@ export const SecretInput = forwardRef( type="button" onClick={() => setVisible(!visible)} disabled={disabled} - data-testid={`${testId}-toggle`} + data-testid={secretInput.toggle(id)} className="absolute right-3 top-1/2 -translate-y-1/2 text-neutral-400 hover:text-neutral-600 dark:hover:text-neutral-300 disabled:opacity-50" aria-label={visible ? 'Hide value' : 'Show value'} > @@ -93,7 +90,7 @@ export const SecretInput = forwardRef(
{error && ( -

+

{error}

)} diff --git a/ushadow/frontend/src/hooks/HOOK_PATTERNS.md b/ushadow/frontend/src/hooks/HOOK_PATTERNS.md new file mode 100644 index 00000000..8fc63043 --- /dev/null +++ b/ushadow/frontend/src/hooks/HOOK_PATTERNS.md @@ -0,0 +1,233 @@ +# Hook Patterns + +> Patterns for separating logic from presentation. Follow these to keep components thin. + +## Core Principle + +**Components render, hooks decide.** + +``` +┌─────────────────┐ ┌─────────────────┐ +│ Component │────▶│ Hook │ +│ (presentation) │ │ (logic) │ +│ │◀────│ │ +│ - JSX layout │ │ - State │ +│ - Styling │ │ - API calls │ +│ - Event wiring │ │ - Computed │ +└─────────────────┘ └─────────────────┘ +``` + +## Pattern 1: Derived State Hook + +Use when: Component needs to make decisions based on multiple inputs. + +**Example**: `useServiceStatus` - decides icon, color, label from raw data. + +```typescript +// hooks/useServiceStatus.ts +export function useServiceStatus( + service: ServiceConfig, + config: Record | undefined, + containerStatus: ContainerStatus | undefined +): ServiceStatusResult { + return useMemo(() => { + // All decision logic here + if (!isConfigured) { + return { state: 'not_configured', label: 'Missing Config', icon: AlertCircle } + } + // ... more decisions + }, [service, config, containerStatus]) +} + +// Component just renders what the hook returns +function ServiceCard({ service, config, status }) { + const { label, icon: Icon, color } = useServiceStatus(service, config, status) + return {label} +} +``` + +## Pattern 2: Data Fetching Hook + +Use when: Component needs server data with loading/error states. + +```typescript +// hooks/useResource.ts +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' + +export function useResource(id: string) { + const queryClient = useQueryClient() + + const query = useQuery({ + queryKey: ['resource', id], + queryFn: () => api.getResource(id), + staleTime: 30_000, + }) + + const updateMutation = useMutation({ + mutationFn: api.updateResource, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['resource', id] }) + }, + }) + + return { + data: query.data, + isLoading: query.isLoading, + error: query.error, + update: updateMutation.mutate, + isUpdating: updateMutation.isPending, + } +} + +// Component stays simple +function ResourcePage({ id }) { + const { data, isLoading, update } = useResource(id) + if (isLoading) return + return +} +``` + +## Pattern 3: Form Logic Hook + +Use when: Form has validation, submission, and error handling. + +```typescript +// hooks/useSettingsForm.ts +import { useForm } from 'react-hook-form' +import { zodResolver } from '@hookform/resolvers/zod' +import { z } from 'zod' + +const schema = z.object({ + apiKey: z.string().min(1, 'Required'), + endpoint: z.string().url('Must be a valid URL'), +}) + +export function useSettingsForm(defaults: SettingsData) { + const form = useForm({ + resolver: zodResolver(schema), + defaultValues: defaults, + }) + + const onSubmit = async (data: SettingsData) => { + await api.saveSettings(data) + toast.success('Saved') + } + + return { + ...form, + onSubmit: form.handleSubmit(onSubmit), + } +} + +// Component wires up the form +function SettingsPage() { + const { control, onSubmit, formState: { errors } } = useSettingsForm(defaults) + + return ( +
+ ( + + )} /> + + ) +} +``` + +## Pattern 4: Action Hook with Confirmation + +Use when: Action needs confirmation dialog and async handling. + +```typescript +// hooks/useDeleteWithConfirm.ts +export function useDeleteWithConfirm(onDelete: (id: string) => Promise) { + const [pendingId, setPendingId] = useState(null) + const [isDeleting, setIsDeleting] = useState(false) + + const requestDelete = (id: string) => setPendingId(id) + const cancelDelete = () => setPendingId(null) + + const confirmDelete = async () => { + if (!pendingId) return + setIsDeleting(true) + try { + await onDelete(pendingId) + } finally { + setIsDeleting(false) + setPendingId(null) + } + } + + return { + pendingId, + isDeleting, + isConfirmOpen: pendingId !== null, + requestDelete, + cancelDelete, + confirmDelete, + } +} +``` + +## Pattern 5: UI State Hook + +Use when: Component has complex UI state (modals, tabs, expansion). + +```typescript +// hooks/useExpandableList.ts +export function useExpandableList(items: T[]) { + const [expandedIds, setExpandedIds] = useState>(new Set()) + + const toggle = (id: string) => { + setExpandedIds(prev => { + const next = new Set(prev) + if (next.has(id)) next.delete(id) + else next.add(id) + return next + }) + } + + const expandAll = () => setExpandedIds(new Set(items.map(i => i.id))) + const collapseAll = () => setExpandedIds(new Set()) + const isExpanded = (id: string) => expandedIds.has(id) + + return { isExpanded, toggle, expandAll, collapseAll } +} +``` + +## When to Extract a Hook + +Extract when you see: +- `useState` + `useEffect` together doing something reusable +- Complex conditional rendering logic (if/else chains) +- API call + loading + error handling +- Multiple pieces of state that change together +- Same logic duplicated across components + +## Hook Composition + +Build complex hooks from simpler ones: + +```typescript +// Compose multiple concerns +export function useServiceManager(serviceId: string) { + const status = useServiceStatus(serviceId) + const { start, stop } = useServiceControls(serviceId) + const { config, updateConfig } = useServiceConfig(serviceId) + const deleteConfirm = useDeleteWithConfirm(deleteService) + + return { + ...status, + start, + stop, + config, + updateConfig, + ...deleteConfirm, + } +} +``` + +## File Size Guide + +- **Hooks**: Max 100 lines each +- **If larger**: Split into multiple hooks and compose them +- **Export**: Always export from `hooks/index.ts` diff --git a/ushadow/frontend/src/testing/ui-contract.ts b/ushadow/frontend/src/testing/ui-contract.ts new file mode 100644 index 00000000..1bca2d67 --- /dev/null +++ b/ushadow/frontend/src/testing/ui-contract.ts @@ -0,0 +1,259 @@ +/** + * UI Contract - Single source of truth for component testid patterns. + * + * This file defines the contract between: + * - React components (which generate data-testid attributes) + * - Playwright POMs (which locate elements by testid) + * - AI agents (which reference these patterns when writing code) + * + * If you change a pattern here, TypeScript will surface breakages. + * + * @example + * // In React component: + * import { modal } from '@/testing/ui-contract' + *
+ * + * // In Playwright POM: + * import { modal } from '../../src/testing/ui-contract' + * page.getByTestId(modal.container('my-modal')) + */ + +// ============================================================================= +// MODAL +// ============================================================================= + +/** + * Modal component - REQUIRED for all modal dialogs. + * + * Import: `import Modal from '@/components/Modal'` + * + * @example + * setIsOpen(false)} + * title="Confirm Action" + * maxWidth="sm" // 'sm' | 'md' | 'lg' | 'xl' | '2xl' + * testId="confirm-delete" + * > + *

Are you sure?

+ * + *
+ * + * @forbidden Do NOT create custom modals with `fixed inset-0` divs. + */ +export const modal = { + /** The root container: `data-testid="my-modal"` */ + container: (id: string) => id, + /** Clickable backdrop: `data-testid="my-modal-backdrop"` */ + backdrop: (id: string) => `${id}-backdrop`, + /** Inner content wrapper: `data-testid="my-modal-content"` */ + content: (id: string) => `${id}-content`, + /** Close X button: `data-testid="my-modal-close"` */ + close: (id: string) => `${id}-close`, +} as const + +// ============================================================================= +// SERVICE CARD +// ============================================================================= + +/** + * ServiceCard component - Displays a service with status, toggle, and config. + * + * Import: `import { ServiceCard } from '@/components/services/ServiceCard'` + * + * @example + * toggleExpanded(service.service_id)} + * onStart={() => startService(service.service_id)} + * onStop={() => stopService(service.service_id)} + * onToggleEnabled={() => toggleEnabled(service.service_id)} + * onStartEdit={() => startEditing(service.service_id)} + * onSave={() => saveConfig(service.service_id)} + * onCancelEdit={cancelEditing} + * onFieldChange={setEditFormField} + * onRemoveField={removeField} + * /> + * + * @note Currently uses `id=` instead of `data-testid=` - migration pending + */ +export const serviceCard = { + /** Card container: `id="service-card-{serviceId}"` */ + container: (serviceId: string) => `service-card-${serviceId}`, + /** Enable/disable toggle: `id="toggle-enabled-{serviceId}"` */ + toggleEnabled: (serviceId: string) => `toggle-enabled-${serviceId}`, +} as const + +// ============================================================================= +// ENV VAR EDITOR +// ============================================================================= + +/** + * EnvVarEditor component - Edit environment variable mappings. + * + * Import: `import EnvVarEditor from '@/components/EnvVarEditor'` + * + * Used for: + * - Docker service configuration (ServicesPage) + * - K8s deployment configuration (DeployToK8sModal) + * - Instance configuration (ServiceConfigsPage) + * + * @example + * setConfig({ ...config, ...updates })} + * /> + */ +export const envVarEditor = { + /** Row container: `data-testid="env-var-editor-{varName}"` */ + container: (varName: string) => `env-var-editor-${varName}`, + /** Map to setting button: `data-testid="map-button-{varName}"` */ + mapButton: (varName: string) => `map-button-${varName}`, + /** Setting path dropdown: `data-testid="map-select-{varName}"` */ + mapSelect: (varName: string) => `map-select-${varName}`, + /** Value text input: `data-testid="value-input-{varName}"` */ + valueInput: (varName: string) => `value-input-${varName}`, +} as const + +// ============================================================================= +// FORM COMPONENTS (from settings/) +// ============================================================================= + +/** + * SecretInput - API key/password input with visibility toggle. + * + * Import: `import { SecretInput } from '@/components/settings/SecretInput'` + * + * @example + * // Standalone + * + * + * @example + * // With react-hook-form + * ( + * + * )} + * /> + */ +export const secretInput = { + container: (id: string) => `secret-input-${id}`, + field: (id: string) => `secret-input-${id}-field`, + toggle: (id: string) => `secret-input-${id}-toggle`, + error: (id: string) => `secret-input-${id}-error`, +} as const + +/** + * SettingField - Generic setting field supporting multiple input types. + * + * Import: `import { SettingField } from '@/components/settings/SettingField'` + * + * @example + * + */ +export const settingField = { + container: (id: string) => `setting-field-${id}`, + input: (id: string) => `setting-field-${id}-input`, + select: (id: string) => `setting-field-${id}-select`, + toggle: (id: string) => `setting-field-${id}-toggle`, + error: (id: string) => `setting-field-${id}-error`, + label: (id: string) => `setting-field-${id}-label`, +} as const + +/** + * SettingsSection - Container for grouping related settings. + * + * Import: `import { SettingsSection } from '@/components/settings/SettingsSection'` + * + * @example + * + * + * + * + */ +export const settingsSection = { + container: (id: string) => `settings-section-${id}`, + header: (id: string) => `settings-section-${id}-header`, + content: (id: string) => `settings-section-${id}-content`, +} as const + +// ============================================================================= +// PAGE-LEVEL PATTERNS +// ============================================================================= + +/** + * Standard page container testid. + * Every page should have a root element with this testid. + */ +export const page = { + container: (name: string) => `${name}-page`, +} as const + +/** + * Tab navigation patterns. + */ +export const tabs = { + button: (id: string) => `tab-${id}`, + panel: (id: string) => `tab-panel-${id}`, +} as const + +/** + * Wizard step patterns. + */ +export const wizard = { + step: (wizardId: string, stepId: string) => `${wizardId}-step-${stepId}`, + nextButton: (wizardId: string) => `${wizardId}-next`, + backButton: (wizardId: string) => `${wizardId}-back`, + submitButton: (wizardId: string) => `${wizardId}-submit`, +} as const + +// ============================================================================= +// GENERIC PATTERNS +// ============================================================================= + +/** + * Confirm dialog patterns. + * Use ConfirmDialog component from '@/components/ConfirmDialog' + */ +export const confirmDialog = { + container: (id: string) => `${id}-dialog`, + title: (id: string) => `${id}-title`, + message: (id: string) => `${id}-message`, + confirmButton: (id: string) => `${id}-confirm`, + cancelButton: (id: string) => `${id}-cancel`, +} as const + +/** + * Action button pattern - for any clickable action within a context. + * + * @example + * + * // Results in: "settings-save" + */ +export const actionButton = (context: string, action: string) => + `${context}-${action}` as const