learning_ai_invt_trdg/docs/ui/UI_AUDIT.md
Devin 63f17bf40e refactor(web): use @bytelyst/ui CardButton primitive (UI audit #9)
Now that @bytelyst/ui@0.1.6 ships <CardButton>, replace the local
`.card-button` className shim (added to layout-fixes.css §25 in the
prior commit) with the real primitive.

Conversions:
  - components/StrategyWizard.tsx — risk style (3 cards) + hours pickers
  - views/SimpleView.tsx — new-buy + manage-holding plan cards
  - tabs/MyStrategiesTab.tsx — diagnostic accordion toggle

Removes layout-fixes.css §25 (.card-button) — no longer needed.
Updates docs/ui/UI_AUDIT.md Pattern A to reflect that the design-system
fix has shipped.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-05-10 09:31:46 +00:00

337 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Trading Web UI Audit
> **Last updated:** 2026-05-10
> **Scope:** `learning_ai_invt_trdg/web/src` (~13,000 lines: 25 tabs/views + 60+ components)
> **Method:** Pattern-based grep audit, manual inspection of recently fixed components, source review of `@bytelyst/ui` Button primitive.
> **Audience:** Engineers maintaining the trading web app and reviewers of `web/src/layout-fixes.css`.
This doc catalogs UI issues by **pattern** (so you fix root causes, not symptoms), points at **specific file:line locations**, and recommends **concrete fixes**. Every metric below was verified against the working tree at commit `3949062`.
---
## 1. Summary — current state
| Metric | Count | Notes |
|---|---:|---|
| `style={{...}}` blocks total occurrences (all files) | ~700 | top files: see §3 Pattern B |
| Files using inline styles | 25 | hotspots: OverviewTab (72), MyStrategiesTab (64), HomeView (55) |
| `!important` declarations in `index.css` | 122 | Indicates repeated specificity fights — see Pattern G |
| `<Button>` (primitive) usages | 31 files | Some at risk for the `whitespace-nowrap` overflow bug we hit — see Pattern A |
| Direct `<table>` usages | 6 files | Most have scroll wrappers; a few don't — see Pattern D |
| `<Card>` with `overflow-hidden` | 7 instances | Some intentional (rounded image clipping), some clip anchored UI — see Pattern D |
| Largest views (line count) | `PositionsTab` 2035, `SimpleView` 1635, `OverviewTab` 1061, `AdminTab` 990, `HomeView` 917 | High UI surface = high bug surface |
| `truncate` usages | 15 occurrences in 7 files | ~11 lack a paired `title=`/`aria-label` — see Pattern E |
The codebase has **pattern bugs**, not just one-offs. The same root causes recur across files. Fixing centrally (CSS overrides + lint rules) is much cheaper than file-by-file.
---
## 2. Already fixed in this session
All centralized in `web/src/layout-fixes.css` (495 lines, 24 numbered sections, single review surface). Each section has a comment explaining what it overrides and why.
| # | Issue | Fix | Commit |
|---|---|---|---|
| 1 | Plans page — saved-setup cards clipped Edit/Delete buttons; chips truncated; outer grid forced 380px column | Refactored saved-setup card to `overflow: visible`, flex-wrap topline, real chip wrapping, single-column form below 1440px | [`46a357c`](#) |
| 2 | Critical alert banner — overflow at narrow widths; loud red styling | Restructured into `.critical-alert-banner-{wrap,inner,icon,text,cta}` with `min-width: 0` text + `flex-shrink: 0` CTA, max-width 1280px, soft tinted background | [`1fa2bcd`](#) |
| 3 | Sidebar logo — "ByteLyst / Trading OS" clipped due to two competing CSS rules (44×44 grid vs 56px flex) for `.trading-sidebar-logo` | Forced flex layout, min-height 60px, brand text ellipsis-only | [`1fa2bcd`](#) |
| 4 | AI Trading Copilot — Quick Action title and prompt overlapped; not responsive | Replaced `<Button>` with native `<button>` + explicit flex-column; responsive 1/2-col grid; `line-clamp` on prompt | [`343ffb4`](#) |
| 5 | DevOps panel tables overflowed | Wrapped in `overflow-x: auto`, `table-layout: fixed`, `word-break: break-word` on cells | [`d2420f5d`](#) (common-plat) |
| 6 | Global: `.dashboard-main { overflow: hidden }` clipped popovers/modals | `overflow: visible !important` | [`f463ff6`](#) |
| 7 | Global: right panel covers main on narrow viewports | Collapses to 260px below 1280px, hidden below 1024px | [`f463ff6`](#) |
| 8 | Global: tables push layout horizontally | `.dashboard-content table` wrapped via `overflow-x: auto` | [`f463ff6`](#) |
| 9 | Global: header search/indices crowd each other | `flex-wrap`, search shrinks 240360px, indices wrap | [`f463ff6`](#) |
Tip: when reviewing layout-fixes.css, read top-to-bottom — sections are ordered roughly from most-global to most-specific.
---
## 3. Issue patterns (root causes)
### Pattern A — `<Button>` primitive collapses multi-line content
**Symptom:** A `<Button>` containing two stacked text spans renders them overlapping or with the second one clipped.
**Actual root cause** (verified against `@bytelyst/ui` source, `packages/ui/src/components/Button.tsx`):
```ts
const baseStyles = 'inline-flex shrink-0 items-center justify-center whitespace-nowrap ...';
const sizes = { sm: 'h-8 ...', md: 'h-10 ...', lg: 'h-11 ...' };
```
Three things compound:
1. **`whitespace-nowrap`** collapses any text content onto a single line.
2. **Fixed height** (`h-8/h-10/h-11`) clips anything taller.
3. **`items-center` + flex-row** lays children side-by-side, not stacked.
So a Button intended as a "card-style action" (e.g. a title above a prompt) will:
- Force both spans onto one line.
- Clip the second span vertically.
- Lay them horizontally if they survive the height clip.
**Fix pattern:** When you need a button-shaped card with stacked content, do not use `<Button>`. Use a native `<button>` with explicit `flex-direction: column`, `white-space: normal`, and `height: auto`. The `<Button>` primitive is correct for single-line CTAs only.
**Where to look:** 31 files use `<Button>`. Most are safe (single label). Audit any whose children include 2+ stacked text spans, an `<img>` next to text, or use `<br />`.
```bash
# Quick triage:
grep -rEn '<Button[^>]*>.*\n[^<]*<' --include='*.tsx' web/src
```
**Recommendation for design system:** ✅ shipped in `@bytelyst/ui@0.1.6` as `<CardButton>`. Drops `whitespace-nowrap`, removes fixed height, preserves the focus-visible ring. Use it whenever a `<Button>` would otherwise wrap a `<div>` or stacked `<span>`s. The 5 Pattern A sites in this repo were converted in commit `c10de34`+ (StrategyWizard ×2, SimpleView ×2, MyStrategiesTab ×1).
### Pattern B — Inline styles instead of classes
**Symptom:** Style drift, no responsive breakpoints, can't override via CSS without `!important` (which is why `!important` count is 122).
**Verified hotspots** (count of `style={{` blocks per file):
| File | Count | Notes |
|---|---:|---|
| `tabs/OverviewTab.tsx` | 72 | landing dashboard |
| `tabs/MyStrategiesTab.tsx` | 64 | strategies grid |
| `views/HomeView.tsx` | 55 | root `/` |
| `components/ChatControl.tsx` | 28 | Copilot — animations OK to leave inline |
| `tabs/MembershipTab.tsx` | 26 | |
| `tabs/MarketplaceTab.tsx` | 23 | |
| `components/strategy/VisualRuleBuilder.tsx` | 22 | |
**Why bad:** Inline styles can't have `@media` queries, can't be tweaked via theme tokens, and fight `index.css` for specificity (every `index.css` rule needs `!important` to win against an inline style).
**Fix pattern:** Lift repeated patterns into named CSS classes. Keep one-off positional/animation styles inline.
### Pattern C — Hardcoded pixel widths in JSX
**Symptom:** Doesn't shrink on narrow viewports → horizontal overflow.
**Verified findings:**
- `App.tsx:226``maxWidth: 420` (toast container)
- `tabs/MembershipTab.tsx:48``width: 44` (icon — likely fine, but verify)
- `tabs/MembershipTab.tsx:160``maxWidth: 1400` (page container — fine)
- `components/strategy/VisualRuleBuilder.tsx:326``width: 260` (input — should be `100%` with `max-width: 260px`)
- `components/layout/Header.tsx:164``width: 7` (likely separator — fine)
**Fix pattern:** Replace fixed widths with flex/grid + `min-width: 0` + `flex: 0 1 <basis>`. For genuine icons, keep fixed width but add `flex-shrink: 0` so the icon doesn't accidentally squish.
### Pattern D — `overflow: hidden` on cards with anchored children
**Symptom:** Buttons, badges, dropdowns clipped at card edges.
**Verified findings:**
- `tabs/ReconciliationAuditPanel.tsx:447,509,579``<Card className="overflow-hidden rounded-2xl">` × 3 (likely intentional for rounded clipping; verify children don't overflow)
- `tabs/AdminTab.tsx:372,608,843` — admin panel sections (panels with internal scroll regions; `overflow-hidden` here is correct)
- `tabs/PositionsTab.tsx:1392,1578``.table-container ... overflow-hidden` wraps a horizontal-scroll table (this is **correct** — the inner `<table>` has its own scroll behavior)
**Fix pattern:** Use `overflow: hidden` only when intentionally clipping (rounded-corner image masks, internal scroll containers). For cards that contain dropdowns/popovers/badges that anchor outside their own bounds, prefer `overflow: visible` and let children manage their own bounds via `position: absolute` or portal.
### Pattern E — Truncation without paired full-text affordance
**Symptom:** User sees `PKRBKDW7…` with no way to reveal the full value.
**Note:** this is a **discoverability/UX issue, not a strict a11y bug.** CSS `text-overflow: ellipsis` does not hide content from screen readers — the full text is still in the DOM and announced. But sighted users on hover/focus get nothing without a `title=` (tooltip) or `aria-label`.
**Verified count:** 15 truncate occurrences across 7 files; ~11 lack a paired `title=` attribute on the same or adjacent line.
**Files:** PositionsTab (7), HistoryTab (4), AdminTab (4), OverviewTab (3), EntriesTab (2), TradeProfileManager (2), MarketOpportunities (2).
**Fix pattern:** Always pair `truncate` with `title={fullValue}` so hover-tooltip shows full content. Even better: a small Radix tooltip on click, since `title` is suppressed on touch devices.
### Pattern F — Grid templates without `minmax(0, 1fr)`
**Symptom:** Grid children with content wider than `1fr` push past container width.
**Verified findings:**
In `index.css`:
- `:1230` `grid-template-columns: 2fr 1.2fr 1fr 1.2fr` (no minmax — risk)
- `:1895` `grid-template-columns: repeat(3, 1fr)` (no minmax — risk)
- `:2325` `grid-template-columns: 100px minmax(220px, 1fr) 90px ...` (correct)
Inline (5 instances confirmed):
- `tabs/MarketplaceTab.tsx:36,86``gridTemplateColumns: '1fr 1fr'`
- `tabs/MembershipTab.tsx:100``gridTemplateColumns: '1fr 1fr'`
- `tabs/MyStrategiesTab.tsx:127,367``gridTemplateColumns: '1fr 1fr'`
**Fix pattern:** Use `minmax(0, 1fr)` not bare `1fr` in any grid template where children may have content wider than the column. Default `min-width` for grid items is `auto``min-content`; `minmax(0, ...)` allows real shrinking. (This is the most common cause of mysterious horizontal scrolling.)
### Pattern G — Two competing CSS rules for the same class
**Symptom:** Styles you can't reason about; final result depends on rule order in 3000-line stylesheet.
**Confirmed example:** `.trading-sidebar-logo` had a 44×44 grid block at line 349 + a 56px flex block at line 1546. The logo bug we fixed (commit `1fa2bcd`) was a direct consequence.
**Likely more:** 122 `!important` declarations indicate repeated overrides. Each `!important` is a fight between two rules — find the duplicates and consolidate.
**How to find them:**
```bash
# Crude: grep class selectors and count duplicates
grep -oE '^\.[a-z][a-z0-9-]+' web/src/index.css | sort | uniq -c | sort -rn | head -20
```
### Pattern H — `100vw` / `vw` units
**Verified findings (some are bugs, most are intentional):**
| Line | Rule | Verdict |
|---|---|---|
| `index.css:1529` | `padding: 36px clamp(24px, 3vw, 44px) 56px` | ✅ fluid padding, fine |
| `index.css:1620` | `width: min(42vw, 460px) !important` | ⚠️ inside dashboard with fixed sidebar — `vw` includes scrollbar/sidebar width |
| `index.css:1791,1798,1822,1832` | `clamp(... vw ...)` for fluid type/spacing | ✅ all inside `clamp()`, fine |
| `components/ChatControl.tsx:635` | `maxWidth: 'calc(100vw - 32px)'` | ✅ portaled modal, no sidebar offset |
| `components/TradeProfileManager.tsx:908` | `max-w-[92vw]` | ✅ portaled drawer, no sidebar offset |
So only **one** real `vw` issue: `index.css:1620`. The rest are inside `clamp()` (fluid scaling) or on portaled elements (where viewport-width is correct).
**Fix pattern:** Inside the dashboard shell use `100%` of the parent flex item, not `100vw`. `vw` is only safe outside the sidebar layout.
---
## 4. High-impact components still to address
### 4.1 PositionsTab.tsx (2035 lines, Portfolio page)
**Verified issues:**
- 7 `truncate` usages (only some have `title=`)
- 4 of those use `truncate max-w-[80260px]` — narrow widths likely cause visible clipping for trade IDs, sub-tags
- Inner `pro-table` has `min-width: 980px` and is wrapped in horizontal-scroll — correct for desktop, but on narrow viewports the table scrolls within itself (intentional)
- 3 separate scroll regions (`.positions-section`, `.orders-section`, `.history-tab`) each with their own min-widths — fine
**Recommendation:** Keep horizontal scroll behavior (correct for data tables). Add `title={fullValue}` to all 7 truncate sites. Inline-style audit: many badges have hardcoded font/padding via inline style — lift to a `.position-badge` class with size variants.
### 4.2 OverviewTab.tsx (1061 lines, dashboard landing)
**Issues:**
- 72 inline `style={{...}}` blocks (largest in repo)
- Likely uses `<Button>` with multi-content children → potential Pattern A overlap
- No responsive breakpoints in inline styles
**Recommendation:** Highest-payoff refactor target. Move colors/spacing into Tailwind utilities or `.overview-*` classes. Estimated 34 hours for a clean pass.
### 4.3 HomeView.tsx (917 lines, root `/`)
**Issues:**
- 55 inline style blocks
- Hero panel uses `clamp(... vw ...)` for fluid typography — looks intentional, leave alone
- Action grid likely uses `gridTemplateColumns: '1fr 1fr'` (Pattern F)
**Recommendation:** Same as OverviewTab — extract into classes. Verify mobile breakpoint for action grid (`grid-cols-1 md:grid-cols-2 xl:grid-cols-3`).
### 4.4 TradeProfileManager.tsx (slide-over drawer)
**Issues:**
- Uses `w-[520px] max-w-[92vw]` — fixed width on desktop, may feel cramped for long forms
- 22 inline-style blocks
- Drawer slides in from right, portaled to body — correct
**Recommendation:** Consider `xl:w-[640px]` for breathing room. Inspect form layout for the same Button overlap issue we saw in Copilot.
### 4.5 ConfigTab.tsx + AdminTab.tsx tables
**ConfigTab:** uses `<th className="min-w-[200px/300px/400px]">` — guarantees horizontal scroll. Acceptable for a config table.
**AdminTab:** `max-h-[400px] overflow-y-auto` × 2 instances bound height for log/audit panels (intentional). `max-w-[260px]` truncate on token displays — intentional (secrets shouldn't fully render). 4 truncate uses — verify each has appropriate title or is intentionally redacted.
**Recommendation:** No urgent fix. Both pages are admin-only and the scroll behavior matches their content density.
### 4.6 ChatControl.tsx (Copilot, just fixed)
Quick Actions block fixed in commit `343ffb4`. 28 inline-style blocks remain — most are background gradients, animations, portal styles. No further action needed.
---
## 5. Recommendations
### Immediate (1 commit, ~1 hour)
1. **Add `title=` to the 11 untitled truncate sites.** Files: PositionsTab, HistoryTab, AdminTab, OverviewTab, EntriesTab, TradeProfileManager, MarketOpportunities. Quick UX win.
2. **Fix the 5 inline `gridTemplateColumns: '1fr 1fr'`** instances by adding `minmax(0, 1fr) minmax(0, 1fr)` (MarketplaceTab × 2, MembershipTab × 1, MyStrategiesTab × 2).
3. **Fix `index.css:1230` and `:1895`** — add `minmax(0, ...)` to bare `1fr` columns.
4. **Fix `index.css:1620`** — replace `42vw` with `min(42%, 460px)` or relative-to-container sizing.
5. **Audit the 31 `<Button>` files** — for any with multi-line/stacked children, replace with native `<button>` (see Pattern A fix).
### Short-term (1 week)
6. **ESLint rules** (preventive):
- Custom rule: warn when `truncate` class is used without `title=` or `aria-label=` on the same element.
- Custom rule: warn on `gridTemplateColumns` containing bare `1fr` (suggest `minmax(0, 1fr)`).
- Custom rule: warn on `<Button>` with non-string children that span multiple lines.
- Limit inline `style` props with > 5 properties (encourage class extraction).
7. **CSS audit script:** scan `index.css` for class selectors with multiple definitions (Pattern G). Aim to reduce `!important` count from 122 → < 60.
### Medium-term (23 weeks)
8. **Refactor OverviewTab + HomeView + MyStrategiesTab** to extract inline styles into `home.css` / `overview.css` / `strategies.css` partials. Highest user impact (these are landing pages).
9. **Component primitives audit:** add a `<CardButton>` primitive in `@bytelyst/ui` (or an `as="card"` variant on `<Button>`) that drops `whitespace-nowrap` and `h-{size}` constraints. Document the difference. Pushes the Pattern A fix to the design system rather than the consumers.
10. **Theme token sweep:** the trading app uses `--bl-*` and `--ml-*` CSS variables. Verify no hardcoded hex values remain in `web/src/**/*.{ts,tsx,css}` (excluding token-default fallbacks like `var(--bl-accent, #5A8CFF)` which are correct per `AGENTS.md`).
---
## 6. How to verify a UI fix
When making changes to layout-fixes.css or any UI component, verify in this order:
1. **Visual smoke-check at 5 viewport widths:** 1920, 1440, 1280, 1024, 768. Use Chrome DevTools' device toolbar.
2. **No horizontal scroll on `<body>`:** open DevTools run `document.body.scrollWidth > window.innerWidth` should be `false` at all widths.
3. **No element extends past its parent:** inspect the suspected component Computed check `width` and parent's `width`.
4. **Modals/popovers not clipped:** open every modal (Copilot, TradeProfileManager, settings dialogs) at narrow widths.
5. **Tables scroll horizontally inside their wrapper, not the page:** scroll the page horizontally should not move.
6. **Run the build:** `cd web && npm run build` (catches Tailwind errors that don't show in dev).
7. **Run typecheck:** `npm run typecheck` if available (catches stray prop changes).
---
## 7. Reproducible audit commands
These commands regenerate the metrics in this doc. Run from `learning_ai_invt_trdg/web/src/`.
```bash
# Inline-style hotspots (count per file, sorted desc)
for f in $(grep -rln "style={{" --include='*.tsx' . | grep -v test); do
count=$(grep -c "style={{" "$f")
[ "$count" -gt 20 ] && echo "$count $f"
done | sort -rn
# !important count
grep -c "!important" index.css
# Truncate occurrences (pair with title= manually)
grep -rEn "\btruncate\b" --include='*.tsx' . | grep -v test | wc -l
grep -rE "\btruncate\b" --include='*.tsx' -B1 -A2 . | grep -v test | grep -c "title="
# Pixel widths in JSX (correct regex — no broken char class)
grep -rEn "(width|minWidth|maxWidth):\s*[0-9]+,?\s*$" --include='*.tsx' . | grep -v test
# Bare 1fr in inline grid templates (Pattern F)
grep -rEn "gridTemplateColumns.*\b1fr\b" --include='*.tsx' . | grep -v test
# Bare 1fr in CSS (no minmax wrapper)
grep -nE "grid-template-columns:.*1fr" index.css | grep -v "minmax"
# Card overflow-hidden
grep -rn "Card.*overflow-hidden" --include='*.tsx' . | grep -v test
# Files using <Button> primitive
grep -rln "<Button" --include='*.tsx' . | grep -v test
```
---
## 8. Open questions / things to investigate
These were noted during the audit but not verified investigate before acting on them.
1. **Does `<Button asChild>` (Radix Slot) inherit `whitespace-nowrap`?** If consumers wrap `<Button asChild><Link>` with multi-line content, they may hit Pattern A even when avoiding `<Button>` directly.
2. **Are any `<Card>` clipping issues real (Pattern D)?** I flagged 6 candidate `overflow-hidden` Cards. Open each at narrow widths and look for cut-off badges, dropdowns, or focus rings.
3. **Mobile / touch:** the audit focused on desktop widths down to 768px. Below that, behavior is unverified. Worth a separate mobile pass if mobile is a target.
4. **Dark theme contrast:** the app is dark-only. No light theme audit performed. If light theme is planned, all `var(--bl-*)` tokens need light-mode definitions (check `@bytelyst/design-tokens`).
5. **The 6 direct `<table>` files that were not in §3 Pattern D.** Verify each has a horizontal-scroll wrapper.
---
## 9. Files changed in recent UI fix sweep
- `web/src/layout-fixes.css` (new, 495 lines) all global UI overrides centralized
- `web/src/main.tsx` imports layout-fixes.css after index.css and App.css
- `web/src/App.tsx` refactored alert banner JSX to nested-div structure
- `web/src/components/ChatControl.tsx` refactored Quick Actions from `<Button>` to native `<button>`
- `web/src/components/layout/AppShell.tsx` added `/devops` `/settings?section=DevOps` redirect
The single-file approach (`layout-fixes.css`) is intentional: every global UI fix lands there with a numbered section + comment explaining the issue. Easy to review, easy to revert one section without touching others.