Files
cameleer-saas/audit/source-code-findings.md
hsiegeln 1133763520 Add SaaS platform UX polish design spec with audit findings
Playwright audit (22 screenshots) + source code audit covering all
platform pages. Spec defines 4 batches: layout fixes (label/value
collision, hardcoded colors), header/navigation (hide server controls,
sidebar active state), error handling & components (DS adoption,
copy-to-clipboard, error states), and polish (tier colors, badges).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 19:39:33 +02:00

434 lines
20 KiB
Markdown

# Cameleer SaaS UI — Source Code Audit Findings
**Audit date:** 2026-04-09
**Scope:** `ui/src/` (platform SPA) + `ui/sign-in/src/` (custom Logto sign-in)
**Design system:** `@cameleer/design-system@0.1.38`
---
## 1. Layout and Styling Patterns
### 1.1 Container Padding/Margin
All three page components use an identical outer wrapper pattern:
```tsx
// DashboardPage.tsx:67, LicensePage.tsx:82, AdminTenantsPage.tsx:60
<div className="space-y-6 p-6">
```
**Verdict:** Consistent across all pages. However, this padding is applied by each page individually rather than by the `Layout` component. If a new page omits `p-6`, the layout will be inconsistent. Consider moving container padding to the `Layout` component wrapping `<Outlet />`.
### 1.2 Use of Design System Components vs Custom HTML
| Component | DashboardPage | LicensePage | AdminTenantsPage |
|-----------|:---:|:---:|:---:|
| Badge | Yes | Yes | Yes |
| Button | Yes | - | - |
| Card | Yes | Yes | Yes |
| DataTable | - | - | Yes |
| EmptyState | Yes | Yes | - |
| KpiStrip | Yes | - | - |
| Spinner | Yes | Yes | Yes |
**Issues found:**
- **LicensePage.tsx:166-170** — Raw `<button>` for "Show token" / "Hide token" toggle instead of DS `Button variant="ghost"`:
```tsx
<button
type="button"
className="text-sm text-primary-400 hover:text-primary-300 underline underline-offset-2 focus:outline-none"
onClick={() => setTokenExpanded((v) => !v)}
>
```
This uses hardcoded Tailwind color classes (`text-primary-400`, `hover:text-primary-300`) instead of design tokens or a DS Button.
- **LicensePage.tsx:174** — Raw `<div>` + `<code>` for token display instead of DS `CodeBlock` (which is available and supports `copyable`):
```tsx
<div className="mt-2 rounded bg-white/5 border border-white/10 p-3 overflow-x-auto">
<code className="text-xs font-mono text-white/80 break-all">
{license.token}
</code>
</div>
```
- **AdminTenantsPage.tsx** — No empty state when `tenants` is empty. The DataTable renders with zero rows but no guidance for the admin.
### 1.3 Card/Section Grouping
- **DashboardPage** uses: KpiStrip + "Tenant Information" Card + "Server Management" Card. Good grouping.
- **LicensePage** uses: "Validity" Card + "Features" Card + "Limits" Card + "License Token" Card. Well-structured.
- **AdminTenantsPage** uses: single Card wrapping DataTable. Appropriate for a list view.
### 1.4 Typography
All pages use the same heading pattern:
```tsx
<h1 className="text-2xl font-semibold text-white">...</h1>
```
**Issue:** `text-white` is hardcoded rather than using a DS color token like `var(--text-primary)`. This will break if the design system ever supports a light theme (the DS has `ThemeProvider` and a theme toggle in the TopBar). The same pattern appears:
- `DashboardPage.tsx:73` — `text-white`
- `LicensePage.tsx:85` — `text-white`
- `AdminTenantsPage.tsx:62` — `text-white`
Similarly, muted text uses `text-white/60` and `text-white/80` throughout:
- `DashboardPage.tsx:96` — `text-white/80`
- `LicensePage.tsx:96,106,109` — `text-white/60`, `text-white`
- `LicensePage.tsx:129` — `text-sm text-white`
- `LicensePage.tsx:150` — `text-sm text-white/60`
These should use `var(--text-primary)` / `var(--text-secondary)` / `var(--text-muted)` from the design system.
### 1.5 Color Token Usage
**Positive:** The sign-in page CSS module (`SignInPage.module.css`) correctly uses DS variables:
```css
color: var(--text-primary); /* line 30 */
color: var(--text-muted); /* line 40 */
background: var(--bg-base); /* line 7 */
font-family: var(--font-body); /* line 20 */
```
**Negative:** The platform SPA pages bypass the design system's CSS variables entirely, using Tailwind utility classes with hardcoded dark-theme colors (`text-white`, `text-white/60`, `bg-white/5`, `border-white/10`, `divide-white/10`).
---
## 2. Interaction Patterns
### 2.1 Button Placement and Order
- **DashboardPage.tsx:81-87** — "Open Server Dashboard" button is top-right (standard). Also repeated inside a Card at line 119-125. Two identical CTAs on the same page is redundant.
- No forms exist in the platform pages. No create/edit/delete operations are exposed in the UI (read-only dashboard).
### 2.2 Confirmation Dialogs for Destructive Actions
- The DS provides `ConfirmDialog` and `AlertDialog` — neither is used anywhere.
- **AdminTenantsPage.tsx:47-57** — Row click silently switches tenant context and navigates to `/`. No confirmation dialog for context switching, which could be disorienting. The user clicks a row in the admin table, and their entire session context changes.
### 2.3 Loading States
All pages use the same loading pattern — centered `<Spinner />` in a fixed-height container:
```tsx
<div className="flex items-center justify-center h-64">
<Spinner />
</div>
```
**Issues:**
- Full-page auth loading screens (LoginPage, CallbackPage, ProtectedRoute, OrgResolver) use inline styles instead of Tailwind:
```tsx
<div style={{ display: 'flex', alignItems: 'center', justifyContent: 'center', minHeight: '100vh' }}>
```
This is inconsistent with the page components which use Tailwind classes.
- The `main.tsx` app bootstrap loading (line 59) also uses inline styles. Six files use this identical inline style pattern — it should be a shared component or consistent class.
- No `Skeleton` components are used anywhere, despite the DS providing `Skeleton`. For the dashboard and license pages which fetch data, skeletons would give better perceived performance than a generic spinner.
### 2.4 Error Handling
- **API client (`api/client.ts`):** Errors are thrown as generic `Error` objects. No toast notifications on failure.
- **LicensePage.tsx:63-69** — Shows `EmptyState` for `isError`. Good.
- **DashboardPage.tsx** — No error state handling at all. If `useTenant()` or `useLicense()` fails, the page renders with fallback `-` values silently. No `isError` check.
- **AdminTenantsPage.tsx** — No error state. If `useAllTenants()` fails, falls through to rendering the table with empty data.
- **OrgResolver.tsx:88-89** — On error, renders `null` (blank screen). The user sees nothing — no error message, no retry option, no redirect. This is the worst error UX in the app.
- No component imports or uses `useToast()` from the DS. Toasts are never shown for any operation.
### 2.5 Empty States
- **DashboardPage.tsx:57-63** — `EmptyState` for no tenant. Good.
- **LicensePage.tsx:54-60** — `EmptyState` for no tenant. Good.
- **LicensePage.tsx:63-69** — `EmptyState` for license fetch error. Good.
- **AdminTenantsPage.tsx** — **Missing.** No empty state when `tenants` array is empty. DataTable will render an empty table body.
---
## 3. Component Usage
### 3.1 DS Imports by File
| File | DS Components Imported |
|------|----------------------|
| `main.tsx` | ThemeProvider, ToastProvider, BreadcrumbProvider, GlobalFilterProvider, CommandPaletteProvider, Spinner |
| `Layout.tsx` | AppShell, Sidebar, TopBar |
| `DashboardPage.tsx` | Badge, Button, Card, EmptyState, KpiStrip, Spinner |
| `LicensePage.tsx` | Badge, Card, EmptyState, Spinner |
| `AdminTenantsPage.tsx` | Badge, Card, DataTable, Spinner + Column type |
| `LoginPage.tsx` | Spinner |
| `CallbackPage.tsx` | Spinner |
| `ProtectedRoute.tsx` | Spinner |
| `OrgResolver.tsx` | Spinner |
| `SignInPage.tsx` (sign-in) | Card, Input, Button, Alert, FormField |
### 3.2 Available but Unused DS Components
These DS components are relevant to the platform UI but unused:
| Component | Could be used for |
|-----------|------------------|
| `AlertDialog` / `ConfirmDialog` | Confirming tenant context switch in AdminTenantsPage |
| `CodeBlock` | License token display (currently raw HTML) |
| `Skeleton` | Loading states instead of spinner |
| `Tooltip` | Badge hover explanations, info about features |
| `StatusDot` | Tenant status indicators |
| `Breadcrumb` / `useBreadcrumb` | Page navigation context (currently empty `[]`) |
| `LoginForm` | Could replace the custom sign-in form (DS already has one) |
| `useToast` | Error/success notifications |
### 3.3 Raw HTML Where DS Components Exist
1. **LicensePage.tsx:166-170** — Raw `<button>` instead of `Button variant="ghost"`
2. **LicensePage.tsx:174-178** — Raw `<div><code>` instead of `CodeBlock`
3. **Layout.tsx:26-62** — Four inline SVG icon components instead of using `lucide-react` icons (the DS depends on lucide-react)
4. **DashboardPage.tsx:95-112** — Manual label/value list with `<div className="flex justify-between">` instead of using a DS pattern (the DS has no explicit key-value list component, so this is acceptable)
### 3.4 Styling Approach
- **Platform SPA pages:** Tailwind CSS utility classes (via class names like `space-y-6`, `p-6`, `flex`, `items-center`, etc.)
- **Sign-in page:** CSS modules (`SignInPage.module.css`) with DS CSS variables
- **Auth loading screens:** Inline `style={{}}` objects
- **No CSS modules** in the platform SPA at all (zero `.module.css` files in `ui/src/`)
This is a three-way inconsistency: Tailwind in pages, CSS modules in sign-in, inline styles in auth components.
---
## 4. Navigation
### 4.1 Sidebar
**File:** `ui/src/components/Layout.tsx:70-118`
The sidebar uses `Sidebar.Section` with `open={false}` and `{null}` children as a workaround to make sections act as navigation links (via `onToggle`). This is a semantic misuse — sections are designed as collapsible containers, not nav links.
```tsx
<Sidebar.Section
icon={<DashboardIcon />}
label="Dashboard"
open={false}
onToggle={() => navigate('/')}
>
{null}
</Sidebar.Section>
```
**Issues:**
- No `active` state is set on any section. The DS supports `active?: boolean` on `SidebarSectionProps` (line 988 of DS types), but it's never passed. The user has no visual indicator of which page they're on.
- `collapsed={false}` is hardcoded with `onCollapseToggle={() => {}}` — the sidebar cannot be collapsed. This is a no-op handler.
- Only three nav items: Dashboard, License, Platform (admin-only). Very sparse.
### 4.2 "Open Server Dashboard"
Two implementations, both identical:
1. **Sidebar footer** (`Layout.tsx:112-116`): `Sidebar.FooterLink` with `window.open('/server/', '_blank', 'noopener')`
2. **Dashboard page** (`DashboardPage.tsx:84`): Primary Button, same `window.open` call
3. **Dashboard page** (`DashboardPage.tsx:120-125`): Secondary Button in a Card, same `window.open` call
Three separate "Open Server Dashboard" triggers on the dashboard. The footer link is good; the two dashboard buttons are redundant.
### 4.3 Breadcrumbs
**File:** `Layout.tsx:124` — `<TopBar breadcrumb={[]} ... />`
Breadcrumbs are permanently empty. The DS provides `useBreadcrumb()` hook (exported, see line 1255 of DS types) that pages can call to set page-specific breadcrumbs, but none of the pages use it. The TopBar renders an empty breadcrumb area.
### 4.4 User Menu / Avatar
**File:** `Layout.tsx:125-126`
```tsx
<TopBar
user={username ? { name: username } : undefined}
onLogout={logout}
/>
```
The TopBar's `user` prop triggers a `Dropdown` with only a "Logout" option. The avatar is rendered by the DS using the `Avatar` component with the user's name.
**Issue:** When `username` is `null` (common if the Logto ID token doesn't have `username`, `name`, or `email` claims), no user indicator is shown at all — no avatar, no logout button. The user has no way to log out from the UI.
---
## 5. Header Bar
### 5.1 Shared TopBar with Server
The platform SPA and the server SPA both use the same `TopBar` component from `@cameleer/design-system`. This means they share identical header chrome.
### 5.2 Irrelevant Controls on Platform Pages
**Critical issue.** The `TopBar` component (DS source, lines 5569-5588 of `index.es.js`) **always** renders:
1. **Status filter pills** (Completed, Warning, Error, Running) — `ButtonGroup` with global filter status values
2. **Time range dropdown** — `TimeRangeDropdown` with presets like "Last 1h", "Last 24h"
3. **Auto-refresh toggle** — "AUTO" / "MANUAL" button
4. **Theme toggle** — Light/dark mode switch
5. **Command palette search** — "Search... Ctrl+K" button
These controls are hardcoded in the DS `TopBar` component. They read from `useGlobalFilters()` and operate on exchange status filters and time ranges — concepts that are **completely irrelevant** to the SaaS platform pages (Dashboard, License, Admin Tenants).
The platform wraps everything in `GlobalFilterProvider` (in `main.tsx:96`), which initializes the filter state, but nothing in the platform UI reads or uses these filters. They are dead UI elements that confuse users.
**Recommendation:** Either:
- The DS should make these controls optional/configurable on `TopBar`
- The platform should use a simpler header component
- The platform should not wrap in `GlobalFilterProvider` / `CommandPaletteProvider` (but this may cause runtime errors if TopBar assumes they exist)
---
## 6. Specific Issues
### 6.1 Label/Value Formatting — "Slugdefault" Concatenation Bug
**Not found in source code.** The source code properly formats label/value pairs with `flex justify-between` layout:
```tsx
// DashboardPage.tsx:96-99
<div className="flex justify-between text-white/80">
<span>Slug</span>
<span className="font-mono">{tenant?.slug ?? '-'}</span>
</div>
```
If "Slugdefault" concatenation is visible in the UI, it's a **rendering/CSS issue** rather than a template bug — the `flex justify-between` may collapse if the container is too narrow, or there may be a DS Card padding issue causing the spans to not separate. The code itself has proper separation.
Similarly for limits on the License page:
```tsx
// LicensePage.tsx:147-155
<span className="text-sm text-white/60">{label}</span>
<span className="text-sm font-mono text-white">{value !== undefined ? value : '—'}</span>
```
Labels and values are in separate `<span>` elements within `flex justify-between` containers. The code is correct.
### 6.2 Badge Colors
**Feature badges (LicensePage.tsx:130-133):**
```tsx
<Badge
label={enabled ? 'Enabled' : 'Not included'}
color={enabled ? 'success' : 'auto'}
/>
```
- Enabled features: `color="success"` (green) — appropriate
- Disabled features: `color="auto"` — this uses the DS's auto-color logic (hash-based). For a disabled/not-included state, `color="error"` or a neutral muted variant would be more appropriate to clearly communicate "not available."
**Tenant status badges (DashboardPage.tsx:102-105, AdminTenantsPage.tsx:24-29):**
```tsx
color={tenant?.status === 'ACTIVE' ? 'success' : 'warning'}
color={row.status === 'ACTIVE' ? 'success' : 'warning'}
```
- ACTIVE: green — appropriate
- Anything else (SUSPENDED, PENDING): yellow/warning — reasonable but SUSPENDED should arguably be `error` (red)
**Tier badges:** Use `tierColor()` function but it's defined differently in each file:
- `DashboardPage.tsx:12-18` maps: enterprise->success, pro->primary, starter->warning
- `LicensePage.tsx:25-33` maps: BUSINESS->success, HIGH->primary, MID->warning, LOW->error
These use **different tier names** (enterprise/pro/starter vs BUSINESS/HIGH/MID/LOW). One is for tenant tiers, the other for license tiers, but the inconsistency suggests either the data model has diverged or one mapping is stale.
### 6.3 Sign-In Page (`ui/sign-in/src/`)
**Positive findings:**
- Uses DS components: `Card`, `Input`, `Button`, `Alert`, `FormField`
- Uses CSS modules with DS CSS variables (`var(--bg-base)`, `var(--text-primary)`, etc.)
- Proper form with `aria-label="Sign in"`, `autoComplete` attributes
- Loading state on submit button via `loading` prop
- Error display via DS `Alert variant="error"`
- Creative rotating subtitle strings — good personality touch
**Issues:**
1. **No `ThemeProvider` wrapper** (`sign-in/src/main.tsx`):
```tsx
createRoot(document.getElementById('root')!).render(
<StrictMode>
<App />
</StrictMode>,
);
```
The sign-in page imports `@cameleer/design-system/style.css` which provides CSS variable defaults, so it works. But the theme toggle won't function, and if the DS ever requires `ThemeProvider` for initialization, this will break.
2. **No `ToastProvider`** — if any DS component internally uses `useToast()`, it will throw.
3. **Hardcoded branding** (`SignInPage.tsx:61`):
```tsx
cameleer3
```
The brand name is hardcoded text, not sourced from configuration.
4. **`React` import unused** (`SignInPage.tsx:1`): `useMemo` and `useState` are imported from `react` but the `import React` default import is absent, which is fine for React 19.
5. **No "forgot password" flow** — the form has username + password only. No recovery link. The DS `LoginForm` component supports `onForgotPassword` and `onSignUp` callbacks.
---
## 7. Architecture Observations
### 7.1 Provider Stack Over-provisioning
`main.tsx` wraps the app in:
```
ThemeProvider > ToastProvider > BreadcrumbProvider > GlobalFilterProvider > CommandPaletteProvider
```
`GlobalFilterProvider` and `CommandPaletteProvider` are server-dashboard concepts (exchange status filters, time range, search). They are unused by any platform page but are required because `TopBar` reads from them internally. This creates coupling between the server's observability UI concerns and the SaaS platform pages.
### 7.2 Route Guard Nesting
The route structure is:
```
ProtectedRoute > OrgResolver > Layout > (pages)
```
`OrgResolver` fetches `/api/me` and resolves tenant context. If it fails (`isError`), it renders `null` — a blank screen inside the Layout shell. This means the sidebar and TopBar render but the content area is completely empty with no explanation.
### 7.3 Unused Import
- `LicensePage.tsx:1` imports `React` and `useState` — `React` import is not needed with React 19's JSX transform, and `useState` is used so that's fine. But `React` as a namespace import isn't used.
### 7.4 DataTable Requires `id` Field
`AdminTenantsPage.tsx:67` passes `tenants` to `DataTable`. The DS type requires `T extends { id: string }`. The `TenantResponse` type has `id: string`, so this works, but the `createdAt` column (line 31) renders the raw ISO timestamp string without formatting — unlike DashboardPage which formats it with `toLocaleDateString()`.
---
## 8. Summary of Issues by Severity
### High Priority
| # | Issue | File(s) | Line(s) |
|---|-------|---------|---------|
| H1 | TopBar shows irrelevant status filters, time range, auto-refresh for platform pages | `Layout.tsx` / DS `TopBar` | 122-128 |
| H2 | OrgResolver error state renders blank screen (no error UI) | `OrgResolver.tsx` | 88-89 |
| H3 | Hardcoded `text-white` colors break light theme | All pages | Multiple |
### Medium Priority
| # | Issue | File(s) | Line(s) |
|---|-------|---------|---------|
| M1 | No active state on sidebar navigation items | `Layout.tsx` | 79-108 |
| M2 | Breadcrumbs permanently empty | `Layout.tsx` | 124 |
| M3 | DashboardPage has no error handling for failed API calls | `DashboardPage.tsx` | 23-26 |
| M4 | AdminTenantsPage missing empty state | `AdminTenantsPage.tsx` | 67-72 |
| M5 | AdminTenantsPage row click silently switches tenant context | `AdminTenantsPage.tsx` | 47-57 |
| M6 | Toasts never used despite ToastProvider being mounted | All pages | - |
| M7 | Raw `<button>` and `<code>` instead of DS components in LicensePage | `LicensePage.tsx` | 166-178 |
| M8 | AdminTenantsPage `createdAt` column renders raw ISO string | `AdminTenantsPage.tsx` | 31 |
| M9 | `tierColor()` defined twice with different tier mappings | `DashboardPage.tsx`, `LicensePage.tsx` | 12-18, 25-33 |
| M10 | "Not included" feature badge uses `color="auto"` instead of muted/neutral | `LicensePage.tsx` | 133 |
### Low Priority
| # | Issue | File(s) | Line(s) |
|---|-------|---------|---------|
| L1 | Three "Open Server Dashboard" buttons/links on dashboard | `Layout.tsx`, `DashboardPage.tsx` | 112-116, 81-87, 119-125 |
| L2 | Inconsistent loading style (inline styles vs Tailwind) | Auth files vs pages | Multiple |
| L3 | No Skeleton loading used (all Spinner) | All pages | - |
| L4 | Sidebar collapse disabled (no-op handler) | `Layout.tsx` | 71 |
| L5 | Sign-in page missing ThemeProvider wrapper | `sign-in/src/main.tsx` | 6-9 |
| L6 | Sign-in page has no forgot-password or sign-up link | `sign-in/src/SignInPage.tsx` | - |
| L7 | Custom SVG icons in Layout instead of lucide-react | `Layout.tsx` | 26-62 |
| L8 | Username null = no logout button visible | `Layout.tsx` | 125-126 |
| L9 | Page padding `p-6` repeated per-page instead of in Layout | All pages | - |