diff --git a/application/ui/docs/adr/001-api-client.md b/application/ui/docs/adr/001-api-client.md new file mode 100644 index 0000000000..07eb8ee427 --- /dev/null +++ b/application/ui/docs/adr/001-api-client.md @@ -0,0 +1,143 @@ +# ADR 001: API Client with openapi-fetch + +## Context + +We needed a type-safe way to communicate with our FastAPI backend. The API contract is defined via OpenAPI spec, and we wanted: + +- Full TypeScript type safety +- Auto-generated types from OpenAPI spec +- React Query integration +- Minimal boilerplate +- Runtime validation + +## Decision + +We use **`openapi-fetch`** with **`openapi-react-query`** for API communication. + +### Implementation + +```typescript +// src/api/client.ts + +import createFetchClient from 'openapi-fetch'; +import createClient from 'openapi-react-query'; + +import type { paths } from './openapi-spec'; + +export const API_BASE_URL = import.meta.env.PUBLIC_API_BASE_URL || ''; +export const fetchClient = createFetchClient({ baseUrl: API_BASE_URL }); +export const $api = createClient(fetchClient); +``` + +### Usage + +**Query Example:** + +```typescript +const { data, isLoading } = $api.useQuery('get', '/api/projects/{project_id}', { + params: { path: { project_id: 'abc-123' } }, +}); +``` + +**Mutation Example:** + +```typescript +const mutation = $api.useMutation('post', '/api/projects'); +mutation.mutate({ + body: { + name: 'New Project', + task: { task_type: 'detection', labels: [...] } + } +}); +``` + +### Type Generation + +Types are auto-generated from the OpenAPI spec: + +```bash +# Fetch latest spec from backend +npm run build:api:download + +# Generate TypeScript types +npm run build:api + +# Combined command +npm run update-spec +``` + +This creates `src/api/openapi-spec.d.ts` with full type definitions for all endpoints. + +## Alternative Considered + +### Axios + Manual Types + +- ❌ Requires manual type definitions +- ❌ Types can drift from API +- ✅ Popular, well-known library + +## Consequences + +### Positive + +- ✅ **Full type safety**: Autocomplete for paths, params, body, response +- ✅ **Single source of truth**: OpenAPI spec drives types +- ✅ **React Query integration**: Built-in caching, optimistic updates, mutations +- ✅ **Low maintenance**: Types auto-update when spec changes +- ✅ **Developer experience**: Instant feedback on API changes + +### Negative + +- ⚠️ **Build step required**: Must run `npm run update-spec` after API changes +- ⚠️ **Backend dependency**: Need running backend to fetch spec +- ⚠️ **Learning curve**: Developers need to understand openapi-fetch patterns + +### Neutral + +- Type generation is fast (~1 second) +- Generated file is committed to version control (easier CI/CD) + +## Implementation Notes + +### Proxy Configuration + +In development, we use Rsbuild proxy to avoid CORS: + +```typescript +// rsbuild.config.ts +server: { + proxy: { + '/api': { + target: 'http://localhost:7860', + changeOrigin: true, + }, + }, +} +``` + +### Error Handling + +```typescript +const { data, error } = $api.useQuery('get', '/api/projects'); + +if (error) { + // error is typed based on OpenAPI error schemas + console.error(error.message); +} +``` + +### Invalidation Patterns + +```typescript +const mutation = $api.useMutation('post', '/api/projects', { + meta: { + invalidateQueries: [['get', '/api/projects']], + }, +}); +``` + +## References + +- [openapi-fetch](https://openapi-ts.pages.dev/openapi-fetch/) +- [openapi-react-query](https://openapi-ts.pages.dev/openapi-react-query/) +- [OpenAPI TypeScript](https://openapi-ts.pages.dev/) diff --git a/application/ui/docs/adr/002-state-management.md b/application/ui/docs/adr/002-state-management.md new file mode 100644 index 0000000000..25a2639a5b --- /dev/null +++ b/application/ui/docs/adr/002-state-management.md @@ -0,0 +1,224 @@ +# ADR 002: State Management with Context Providers + +## Context + +We needed a state management solution that: + +- Handles complex domain logic (annotations, zoom, selections) +- Avoids prop drilling +- Works well with React Query for server state +- Keeps components decoupled +- Supports multiple independent state domains + +## Decision + +Use **React Context API with custom providers** for domain-specific state, and **React Query** for server state. + +### Architecture + +``` +State Layer: +├── Server State (React Query) +│ └── API data, caching, mutations +└── Client State (Context Providers) + ├── AnnotationActionsProvider (CRUD operations) + ├── AnnotationVisibilityProvider (UI state) + ├── SelectAnnotationProvider (selection state) + ├── AnnotatorProvider (tool state) + └── ZoomProvider (zoom/pan state) +``` + +## Implementation + +### Provider Pattern + +```typescript +// annotation-actions-provider.component.tsx +interface AnnotationsContextValue { + annotations: Annotation[]; + addAnnotations: (shapes: Shape[]) => void; + deleteAnnotations: (ids: string[]) => void; + updateAnnotations: (annotations: Annotation[]) => void; +} + +const AnnotationsContext = createContext(null); + +export const AnnotationActionsProvider = ({ children, mediaItem }) => { + const [localAnnotations, setLocalAnnotations] = useState([]); + + const addAnnotations = (shapes: Shape[]) => { + setLocalAnnotations(prev => [...prev, ...shapes.map(toAnnotation)]); + }; + + return ( + + {children} + + ); +}; + +export const useAnnotationActions = () => { + const context = useContext(AnnotationsContext); + if (!context) throw new Error('Must use within provider'); + return context; +}; +``` + +### Provider Composition + +```tsx +// media-preview.component.tsx + + + + + {/* Components access any provider via hooks */} + + + + +``` + +## Provider Responsibilities + +### AnnotationActionsProvider + +**Purpose**: Manages annotation data and CRUD operations +**State**: `localAnnotations[]`, `isDirty` +**Actions**: `addAnnotations()`, `updateAnnotations()`, `deleteAnnotations()`, `submitAnnotations()` +**Syncs with**: Server via React Query + +### AnnotationVisibilityProvider + +**Purpose**: UI visibility state +**State**: `isVisible`, `isFocussed` +**Actions**: `toggleVisibility()`, `toggleFocus()` +**Pure UI**: No server interaction + +### SelectAnnotationProvider + +**Purpose**: Selection state +**State**: `selectedAnnotations: Set` +**Actions**: `setSelectedAnnotations()`, `toggleSelection()` +**Supports**: Multi-select, click handlers + +### ZoomProvider + +**Purpose**: Canvas zoom/pan transformations +**State**: `scale`, `translate`, `canvasSize` +**Actions**: `setZoom()`, `fitToScreen()`, `zoomIn()`, `zoomOut()` +**Math**: Transform calculations + +## Alternatives Considered + +### 1. Zustand + +- ❌ Another dependency +- ❌ Less explicit than Context +- ✅ Simpler API than Redux +- ✅ Good performance + +### 2. Props Drilling + +- ❌ Unmaintainable for deep hierarchies +- ❌ Violates component boundaries +- ✅ Simple, explicit +- ✅ No magic + +## Consequences + +### Positive + +- ✅ **Scoped state**: Each provider owns its domain +- ✅ **No prop drilling**: Components access state directly +- ✅ **Testable**: Providers can be tested in isolation +- ✅ **Composable**: Mix and match providers as needed +- ✅ **Type-safe**: Full TypeScript support +- ✅ **React-native**: Uses standard React patterns + +### Negative + +- ⚠️ **Provider hell**: Deep nesting can be verbose +- ⚠️ **Re-render optimization**: Need `useMemo`/`useCallback` carefully +- ⚠️ **No DevTools**: Unlike Redux, no time-travel debugging +- ⚠️ **Testing setup**: Each test needs provider wrapper + +### Neutral + +- Context API is built-in (no extra dependencies) +- Performance is good enough for our use case +- Can migrate to Zustand/Redux later if needed + +## Best Practices + +### 1. Single Responsibility + +Each provider manages ONE domain: + +```typescript +// ✅ Good +AnnotationVisibilityProvider; // Only visibility state + +// ❌ Bad +AnnotationProvider; // Too broad, mixes concerns +``` + +### 2. Custom Hooks + +Always provide a custom hook: + +```typescript +export const useAnnotationActions = () => { + const context = useContext(AnnotationsContext); + if (!context) throw new Error('Provider missing'); + return context; +}; +``` + +### 3. Minimize Re-renders + +```typescript +const value = useMemo( + () => ({ + annotations, + addAnnotations, + deleteAnnotations, + }), + [annotations] +); // Only recreate if annotations change +``` + +### 4. Provider Placement + +Place providers as close as possible to consumers: + +```typescript +// ✅ Scoped to feature + + + + + + +// ❌ Too global + + {/* Unnecessary for non-annotation pages */} + + + +``` + +## Migration Path + +If we outgrow Context, migration path: + +1. Keep provider interfaces unchanged +2. Swap Context implementation with Zustand/Redux +3. Consumers continue using custom hooks +4. No component changes needed + +## References + +- [React Context API](https://react.dev/reference/react/createContext) +- [Context Performance](https://react.dev/reference/react/useMemo#skipping-expensive-recalculations) +- [Composition Pattern](https://react.dev/learn/passing-data-deeply-with-context) diff --git a/application/ui/docs/adr/003-testing-strategy.md b/application/ui/docs/adr/003-testing-strategy.md new file mode 100644 index 0000000000..59ef7e6560 --- /dev/null +++ b/application/ui/docs/adr/003-testing-strategy.md @@ -0,0 +1,305 @@ +# ADR 003: Testing Strategy + +## Context + +We needed a comprehensive testing strategy that: + +- Catches bugs early in development +- Validates user workflows end-to-end +- Runs fast in CI/CD pipeline +- Provides confidence for refactoring +- Balances coverage with maintainability + +## Decision + +Use a **three-tier testing pyramid**: + +1. **Unit Tests** (Vitest) - Component logic, utilities, hooks +2. **Component Tests** (Playwright Component) - Visual components, interactions +3. **E2E Tests** (Playwright) - User workflows, critical paths + +### Testing Pyramid + +``` + /\ + / \ E2E (Playwright) + / \ - User workflows + /------\ - Critical paths + / \ - ~10-20 tests + /----------\ + / Component \ (Playwright Component) + / Tests \ - UI interactions + /--------------\ - ~30-50 tests +/ \ +/ Unit Tests \ (Vitest) +/ (Vitest) \ - Logic, utils, hooks +/------------------\ - ~100+ tests +``` + +## Implementation + +### 1. Unit Tests (Vitest) + +**Purpose**: Test business logic, utilities, custom hooks in isolation + +```typescript +// src/hooks/use-zoom.test.ts + +import { act, renderHook } from '@testing-library/react'; + +import { useZoom } from './use-zoom'; + +describe('useZoom', () => { + it('should initialize with default scale', () => { + const { result } = renderHook(() => useZoom()); + expect(result.current.scale).toBe(1); + }); + + it('should zoom in by 10%', () => { + const { result } = renderHook(() => useZoom()); + act(() => result.current.zoomIn()); + expect(result.current.scale).toBe(1.1); + }); + + it('should not zoom beyond max scale', () => { + const { result } = renderHook(() => useZoom({ maxScale: 5 })); + act(() => { + for (let i = 0; i < 100; i++) result.current.zoomIn(); + }); + expect(result.current.scale).toBe(5); + }); +}); +``` + +**Run**: `npm test` + +### 2. Component Tests (Playwright Component) + +**Purpose**: Test component rendering, user interactions, visual states + +```typescript +// src/components/annotation-labels.test.tsx +import { test, expect } from '@playwright/experimental-ct-react'; +import { AnnotationLabels } from './annotation-labels.component'; + +test('should render label with correct text', async ({ mount }) => { + const component = await mount( + + ); + + await expect(component.getByText('Person')).toBeVisible(); +}); +``` + +**Run**: `npm run test:component` + +### 3. E2E Tests (Playwright) + +**Purpose**: Test complete user workflows across the application + +```typescript +// tests/e2e/annotation-workflow.spec.ts + +import { expect, test } from '@playwright/test'; + +test.describe('Annotation Workflow', () => { + test('should create, edit, and delete annotation', async ({ page }) => { + await page.goto('/projects/123/annotate'); + + // Create annotation + await page.getByRole('button', { name: 'Rectangle' }).click(); + await page.locator('canvas').click({ position: { x: 100, y: 100 } }); + await page.locator('canvas').click({ position: { x: 300, y: 300 } }); + + // Add label + await page.getByRole('combobox', { name: /label/i }).fill('Person'); + await page.keyboard.press('Enter'); + + // Verify annotation exists + await expect(page.getByText('Person')).toBeVisible(); + + // Submit annotations + await page.getByRole('button', { name: 'Save' }).click(); + await expect(page.getByText('Saved successfully')).toBeVisible(); + + // Delete annotation + await page.getByText('Person').click(); + await page.keyboard.press('Delete'); + await expect(page.getByText('Person')).not.toBeVisible(); + }); +}); +``` + +**Run**: `npm run test:e2e` + +## Testing Configuration + +### vitest.config.ts + +```typescript +export default defineConfig({ + test: { + environment: 'jsdom', + setupFiles: ['./src/setup-tests.ts'], + coverage: { + provider: 'v8', + reporter: ['text', 'json', 'html'], + exclude: ['**/*.test.{ts,tsx}', '**/mocks/**', '**/*.d.ts'], + }, + }, +}); +``` + +### playwright.config.ts + +```typescript +export default defineConfig({ + testDir: './tests', + use: { + baseURL: 'http://localhost:3000', + screenshot: 'only-on-failure', + video: 'retain-on-failure', + }, + webServer: { + command: 'npm run dev', + port: 3000, + reuseExistingServer: !process.env.CI, + }, +}); +``` + +## Alternatives Considered + +### 1. Jest + +- ❌ Slower than Vitest +- ❌ More configuration required +- ✅ Industry standard, more resources +- ✅ Mature ecosystem + +### 2. Cypress + +- ❌ Slower test execution than Playwright +- ❌ Less powerful component testing +- ✅ Great debugging UI +- ✅ Time-travel debugging + +## Consequences + +### Positive + +- ✅ **Fast feedback**: Vitest runs in <2s for unit tests +- ✅ **Confidence**: E2E tests catch integration bugs +- ✅ **DX**: Playwright UI mode for debugging +- ✅ **Type-safe**: Full TypeScript support +- ✅ **CI-friendly**: Parallel execution, retries +- ✅ **Real browser**: Playwright uses actual Chromium/Firefox + +### Negative + +- ⚠️ **Maintenance**: E2E tests can be brittle +- ⚠️ **Slow E2E**: Full workflows take 30s-2min +- ⚠️ **Learning curve**: Two test frameworks to learn +- ⚠️ **CI cost**: E2E tests require more resources + +## Testing Best Practices + +### 1. Test User Behavior, Not Implementation + +```typescript +// ✅ Good - Tests behavior +test('should add label when Enter is pressed', async ({ page }) => { + await page.getByRole('textbox').fill('Person'); + await page.keyboard.press('Enter'); + await expect(page.getByText('Person')).toBeVisible(); +}); + +// ❌ Bad - Tests implementation +test('should call addLabel function', () => { + const addLabel = vi.fn(); + render(); + // ... test calls addLabel +}); +``` + +### 2. Use Data-Testid Sparingly + +```typescript +// ✅ Prefer semantic selectors +await page.getByRole('button', { name: 'Save' }); +await page.getByLabel('Label name'); + +// ⚠️ Use data-testid only when necessary +await page.getByTestId('annotation-canvas'); +``` + +### 3. Arrange-Act-Assert Pattern + +```typescript +test('should zoom in when button clicked', () => { + // Arrange + const { result } = renderHook(() => useZoom()); + + // Act + act(() => result.current.zoomIn()); + + // Assert + expect(result.current.scale).toBe(1.1); +}); +``` + +### 4. Mock External Dependencies + +```typescript +// setup-tests.ts +vi.mock('./api/client', () => ({ + fetchAnnotations: vi.fn(() => Promise.resolve([])), +})); +``` + +### 5. Test Error States + +```typescript +test('should show error toast on upload failure', async ({ page }) => { + await page.route('**/api/media', (route) => route.abort()); + await page.getByRole('button', { name: 'Upload' }).click(); + await expect(page.getByText(/upload failed/i)).toBeVisible(); +}); +``` + +## Coverage Targets + +- **Unit Tests**: >80% coverage for utilities, hooks +- **Component Tests**: Critical user-facing components +- **E2E Tests**: All primary user workflows + +### Running Coverage + +```bash +npm run test:coverage +``` + +## CI Integration + +```yaml +# .github/workflows/test.yml +- name: Run unit tests + run: npm test -- --coverage + +- name: Run component tests + run: npm run test:component + +- name: Run E2E tests + run: npm run test:e2e +``` + +## References + +- [Vitest Documentation](https://vitest.dev/) +- [Playwright Documentation](https://playwright.dev/) +- [Testing Library](https://testing-library.com/) +- [Playwright Component Testing](https://playwright.dev/docs/test-components) diff --git a/application/ui/docs/adr/004-ui-packages.md b/application/ui/docs/adr/004-ui-packages.md new file mode 100644 index 0000000000..a7044893d6 --- /dev/null +++ b/application/ui/docs/adr/004-ui-packages.md @@ -0,0 +1,201 @@ +# ADR 004: UI Packages Architecture + +## Context + +We needed a UI component system that: + +- Provides accessible, production-ready components +- Maintains consistency across the application +- Supports theming and customization +- Avoids reinventing common patterns +- Integrates with our React/TypeScript stack + +Additionally, we wanted to share this system with related projects (Geti) and organize shared utilities. + +## Decision + +Use **@geti/ui** (Adobe React Spectrum based) as our primary component library, organized as a monorepo with separate packages: + +``` +ui/ +├── packages/ +│ ├── ui/ # @geti/ui - Adobe Spectrum components +│ ├── smart-tools/ # @geti/smart-tools - AI annotation utilities +│ └── config/ # Shared build/lint configuration +``` + +### Package Responsibilities + +#### @geti/ui + +- **Purpose**: Core UI component library +- **Based on**: Adobe React Spectrum +- **Contains**: Buttons, Forms, Tables, Dialogs, Navigation, Layout +- **Accessibility**: ARIA compliant out-of-the-box +- **Theming**: CSS variables for customization + +#### @geti/smart-tools + +- **Purpose**: AI-powered annotation utilities +- **Contains**: Auto-segmentation, object detection helpers +- **Independent**: Can be used without @geti/ui + +#### packages/config + +- **Purpose**: Shared build configuration +- **Contains**: ESLint, TypeScript, Rsbuild configs +- **DRY**: Single source of truth for tooling + +## Implementation + +### Package Structure + +``` +packages/ui/ +├── package.json +├── src/ +│ ├── index.ts # Public API +│ ├── components/ +│ │ ├── button/ +│ │ │ ├── button.component.tsx +│ │ │ ├── button.test.tsx +│ │ │ └── index.ts +│ │ ├── form/ +│ │ ├── table/ +│ │ └── ... +│ ├── hooks/ +│ │ ├── use-theme.ts +│ │ └── use-media-query.ts +│ └── styles/ +│ ├── theme.scss +│ └── variables.scss +``` + +### Usage in Application + +```typescript +// src/features/annotator/toolbar.component.tsx +import { + Button, + ActionButton, + Form, + TextField, + TooltipTrigger, + Tooltip, +} from '@geti/ui'; + +export const Toolbar = () => { + return ( +
+ + + + + Draw Rectangle + + +
+ + + +
+ ); +}; +``` + +### Workspace Configuration + +```json +// package.json +{ + "workspaces": ["packages/*", "src"], + "dependencies": { + "@geti/ui": "workspace:*", + "@geti/smart-tools": "workspace:*" + } +} +``` + +### Positive + +- ✅ **Accessibility**: WCAG 2.1 AA compliant out of the box +- ✅ **Productivity**: Focus on features, not reinventing buttons +- ✅ **Consistency**: Same components across app +- ✅ **Type-safe**: Full TypeScript support +- ✅ **Maintained**: Adobe maintains the library +- ✅ **Cross-project**: Share with Geti and other tools + +### Negative + +- ⚠️ **Vendor lock-in**: Hard to migrate away from Spectrum +- ⚠️ **Bundle size**: ~150KB gzipped (reasonable but not tiny) +- ⚠️ **Customization limits**: Some components hard to style deeply +- ⚠️ **Learning curve**: Different API than HTML elements + +### Neutral + +- Monorepo adds complexity (but provides organization) +- Adobe design language may not match brand +- Community is smaller than MUI/Chakra + +## Customization + +### Theme Variables + +```scss +// packages/ui/src/styles/theme.scss +:root { + // Colors + --primary-color: #0078d4; + --secondary-color: #6c757d; + --success-color: #28a745; + --danger-color: #dc3545; + + // Spacing + --spacing-xs: 4px; + --spacing-sm: 8px; + --spacing-md: 16px; + --spacing-lg: 24px; + + // Typography + --font-family: 'Inter', sans-serif; + --font-size-sm: 12px; + --font-size-md: 14px; + --font-size-lg: 16px; +} +``` + +### Custom Components + +When Spectrum doesn't provide what we need: + +```typescript +// packages/ui/src/components/annotation-label/ +import { useButton } from '@react-aria/button'; +import { useObjectRef } from '@react-aria/utils'; + +export const AnnotationLabel = ({ label, onDelete }) => { + // Use React Aria hooks for accessibility + const ref = useObjectRef(null); + const { buttonProps } = useButton({ onPress: onDelete }, ref); + + return ( +
+ {label.name} + +
+ ); +}; +``` + +## References + +- [Adobe React Spectrum](https://react-spectrum.adobe.com/) +- [React Aria](https://react-spectrum.adobe.com/react-aria/) (Headless version) +- [Spectrum Design System](https://spectrum.adobe.com/) +- [WCAG 2.1 Guidelines](https://www.w3.org/WAI/WCAG21/quickref/) +- [npm Workspaces](https://docs.npmjs.com/cli/v7/using-npm/workspaces) diff --git a/application/ui/docs/adr/005-feature-structure.md b/application/ui/docs/adr/005-feature-structure.md new file mode 100644 index 0000000000..223cc5bd07 --- /dev/null +++ b/application/ui/docs/adr/005-feature-structure.md @@ -0,0 +1,370 @@ +# ADR 005: Feature-Based Architecture + +## Context + +We needed a scalable folder structure that: + +- Supports growth from small to large application +- Makes feature boundaries explicit +- Reduces cognitive load when navigating codebase +- Avoids deeply nested folder hierarchies +- Enables team autonomy (multiple devs working on different features) + +Traditional MVC/layered architectures become hard to navigate as apps grow: + +``` +❌ Hard to scale +src/ +├── components/ # 100+ components mixed together +├── hooks/ # Which hook belongs to which feature? +├── services/ # Tightly coupled services +└── utils/ # Grab-bag of utilities +``` + +## Decision + +Use a **feature-based architecture** where code is organized by domain, not by type: + +``` +✅ Scales well +src/ +├── features/ # Domain features +│ ├── annotator/ # Everything for annotation +│ ├── dataset/ # Everything for dataset management +│ ├── inference/ # Everything for inference +│ └── project/ # Everything for project management +├── components/ # Truly shared components +├── hooks/ # Truly shared hooks +├── api/ # API client (shared) +└── constants/ # Global constants +``` + +## Feature Structure + +Each feature is a **self-contained module** with its own: + +``` +features/annotator/ +├── index.ts # Public API (exports) +├── annotator.provider.tsx # Feature-level state +├── annotator.routes.tsx # Feature routes +├── components/ +│ ├── canvas/ +│ │ ├── canvas.component.tsx +│ │ ├── canvas.component.scss +│ │ └── canvas.test.tsx +│ ├── toolbar/ +│ ├── shapes/ +│ └── labels/ +├── hooks/ +│ ├── use-annotation-actions.ts +│ ├── use-draw-shape.ts +│ └── use-select-annotation.ts +├── providers/ +│ ├── annotation-actions-provider.tsx +│ ├── annotation-visibility-provider.tsx +│ └── zoom-provider.tsx +├── utils/ +│ ├── shape-calculations.ts +│ ├── coordinate-transform.ts +│ └── polylabel.ts +├── types/ +│ ├── annotation.ts +│ └── shape.ts +└── constants/ + ├── tools.ts + └── colors.ts +``` + +## Implementation + +### Feature Public API + +```typescript +// features/annotator/index.ts +// Only export what other features need + +export { AnnotatorProvider } from './annotator.provider'; +export { AnnotatorRoutes } from './annotator.routes'; +export { useAnnotationActions } from './hooks/use-annotation-actions'; +export type { Annotation, Shape } from './types'; + +// Internal components NOT exported +// - Canvas, Toolbar, Labels, etc. +``` + +### Feature Routes + +```typescript +// features/annotator/annotator.routes.tsx +import { Route } from 'react-router-dom'; +import { Canvas } from './components/canvas/canvas.component'; + +export const AnnotatorRoutes = () => ( + <> + } /> + +); +``` + +### Root Router + +```typescript +// src/router.tsx +import { AnnotatorRoutes } from './features/annotator'; +import { DatasetRoutes } from './features/dataset'; +import { ProjectRoutes } from './features/project'; + +export const Router = () => ( + + + {AnnotatorRoutes()} + {DatasetRoutes()} + {ProjectRoutes()} + + +); +``` + +## Feature Boundaries + +### ✅ Good - Features Are Independent + +```typescript +// features/dataset/components/media-gallery.tsx + +import { fetchMediaItems } from '@/api/media'; +import { Button } from '@geti/ui'; +import { useQuery } from '@tanstack/react-query'; + +// Only imports from: +// 1. External libraries +// 2. Shared UI components +// 3. API client +// NO imports from other features +``` + +### ⚠️ Acceptable - Features Communicate via API + +```typescript +// features/annotator/components/canvas.tsx + +import { fetchAnnotations } from '@/api/annotations'; +import { useQuery } from '@tanstack/react-query'; + +// Features don't talk to each other directly +// They talk via API (single source of truth) +``` + +### ❌ Bad - Features Depend on Each Other + +```typescript +// features/annotator/components/canvas.tsx + +import { MediaGallery } from '@/features/dataset/components/media-gallery'; + +// ❌ Creates tight coupling between features +``` + +## Shared Code Guidelines + +### When to Share + +Code belongs in `/src/components` or `/src/hooks` if: + +1. **Used by 3+ features** (not just 2) +2. **Generic and reusable** (not domain-specific) +3. **Stable interface** (won't change per feature needs) + +```typescript +// ✅ Truly shared - Used by annotator, dataset, inference +src / components / loading - + spinner / + // ✅ Truly shared - Used everywhere + src / + hooks / + use - + debounce.ts; + +// ❌ Not shared - Only used in annotator +features / annotator / hooks / use - draw - rectangle.ts; +``` + +### Shared Components + +``` +src/components/ +├── error-boundary/ # Used everywhere for error handling +├── empty-state/ # Used in dataset, inference, project +└── confirmation-dialog/ # Used in dataset, project +``` + +### Feature-Specific Components + +``` +features/annotator/components/ +├── canvas/ # Only annotator needs canvas +├── shape-editor/ # Only annotator needs shape editing +└── polygon-tool/ # Only annotator needs polygon tool +``` + +## Dependency Rules + +``` +┌─────────────────┐ +│ Features │ ← Can import from shared +│ (annotator) │ ← Can import from api/ +│ │ ← Can import from @geti/ui +└─────────────────┘ ← CANNOT import from other features + ↓ +┌─────────────────┐ +│ Shared Code │ ← Can import from api/ +│ (components/) │ ← Can import from @geti/ui +│ │ ← CANNOT import from features/ +└─────────────────┘ + ↓ +┌─────────────────┐ +│ API Client │ ← Can only import from openapi-fetch +│ (api/) │ ← CANNOT import from features or components +└─────────────────┘ +``` + +## Feature Communication Patterns + +### 1. Via URL Parameters + +```typescript +// features/dataset/components/media-gallery.tsx +const navigate = useNavigate(); + +const handleAnnotate = (mediaId: string) => { + navigate(`/projects/${projectId}/annotate?media=${mediaId}`); +}; + +// features/annotator/components/canvas.tsx +const [searchParams] = useSearchParams(); +const mediaId = searchParams.get('media'); +``` + +### 2. Via Shared API State (React Query) + +```typescript +// Both features read from same cache +const { data: project } = useQuery({ + queryKey: ['projects', projectId], + queryFn: () => fetchProject(projectId), +}); +``` + +### 3. Via Events (Rare) + +```typescript +// Only for cross-cutting concerns like notifications +window.dispatchEvent( + new CustomEvent('annotation-saved', { + detail: { annotationId: '123' }, + }) +); +``` + +## Architecture consequences + +### Positive + +- ✅ **Easy to find code**: Everything for a feature is in one place +- ✅ **Easy to delete**: Remove feature by deleting one folder +- ✅ **Team autonomy**: Teams can own entire features +- ✅ **Reduced coupling**: Features can't accidentally depend on each other +- ✅ **Faster onboarding**: Clear feature boundaries +- ✅ **Parallel development**: Multiple devs work on different features without conflicts + +### Negative + +- ⚠️ **Duplication risk**: Features might duplicate code instead of sharing +- ⚠️ **Judgment required**: "Is this truly shared?" decisions +- ⚠️ **Refactoring cost**: Moving code between shared/feature can be tedious +- ⚠️ **Import paths**: More `../../` or need path aliases + +### Neutral + +- Not a silver bullet - still requires discipline +- Works best with domain-driven thinking +- May feel unfamiliar to developers used to MVC + +## Best Practices + +### 1. Colocation + +Place code next to where it's used: + +``` +✅ features/annotator/hooks/use-draw-rectangle.ts +❌ src/hooks/use-draw-rectangle.ts (if only annotator uses it) +``` + +### 2. Limit Feature Scope + +Features should be **cohesive** but not too broad: + +``` +✅ features/annotator/ # Focused on annotation +✅ features/dataset/ # Focused on media management + +❌ features/app/ # Too broad +❌ features/utils/ # Not a feature +``` + +### 3. Public API Discipline + +Only export what's truly needed: + +```typescript +// ✅ Minimal public API +export { AnnotatorProvider, useAnnotationActions }; + +// ❌ Exposing internals +export { Canvas, Toolbar, DrawRectangle, ... }; // Too much +``` + +### 4. Use Path Aliases + +```json +// tsconfig.json +{ + "compilerOptions": { + "paths": { + "@/api/*": ["./src/api/*"], + "@/features/*": ["./src/features/*"], + "@/components/*": ["./src/components/*"] + } + } +} +``` + +```typescript +// ✅ Clean imports +import { Button } from '@geti/ui'; +import { fetchAnnotations } from '@/api/annotations'; +import { useAnnotationActions } from '@/features/annotator'; + +// ❌ Messy imports +import { Button } from '../../../packages/ui'; +import { fetchAnnotations } from '../../../api/annotations'; +``` + +## Migration Strategy + +Moving from layered to feature-based: + +1. **Identify features** (annotator, dataset, inference, project) +2. **Create feature folders** (`features/annotator/`) +3. **Move components one at a time** +4. **Update imports** as you go +5. **Extract shared code last** (wait until patterns emerge) + +## References + +- [Feature-Sliced Design](https://feature-sliced.design/) +- [Domain-Driven Design Frontend](https://khalilstemmler.com/articles/client-side-architecture/introduction/) +- [Bulletproof React](https://github.com/alan2207/bulletproof-react) +- [Screaming Architecture](https://blog.cleancoder.com/uncle-bob/2011/09/30/Screaming-Architecture.html) diff --git a/application/ui/docs/adr/README.md b/application/ui/docs/adr/README.md new file mode 100644 index 0000000000..6d6df442a5 --- /dev/null +++ b/application/ui/docs/adr/README.md @@ -0,0 +1,17 @@ +# Architectural Decision Records (ADR) + +This directory contains records of architectural decisions made in the Geti Tune UI project. + +## What is an ADR? + +An Architectural Decision Record (ADR) captures an important architectural decision made along with its context and consequences. + +## Index + +| ADR | Title | Status | +| --------------------------------- | --------------------------------------- | -------- | +| [001](./001-api-client.md) | API Client with openapi-fetch | Accepted | +| [002](./002-state-management.md) | State Management with Context Providers | Accepted | +| [003](./003-testing-strategy.md) | Testing Strategy | Accepted | +| [004](./004-ui-packages.md) | Shared UI Packages (@geti/ui) | Accepted | +| [005](./005-feature-structure.md) | Feature-Based Architecture | Accepted |