Skip to content

chore(react): impeccable audit — drift hex, loader, side-stripe, edit feedback, icon-family consolidation, polish #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
romer8 wants to merge 5 commits into
base: feature/tethysdash-mcp-server
Choose a base branch
from
Draft

chore(react): impeccable audit — drift hex, loader, side-stripe, edit feedback, icon-family consolidation, polish #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48dd55c
chore(react): impeccable audit — drift hex, loader, side-stripe, edit…
romer8 May 29, 2026
ce845be
chore(react): icon families 12 → 1 (Bootstrap Icons) + eslint guardrail
romer8 May 29, 2026
ec569cc
chore(viz): impeccable visualizations audit — Plotly theme, Card quie…
romer8 May 29, 2026
ad40031
chore(viz): map popup table aligned with DataTable.js typeset
romer8 May 29, 2026
238842f
chore(landing): header de-saturate + dashboard-card de-identical
romer8 May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions .impeccable/critique/2026-05-28T20-25-19Z__reactapp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
---
target: reactapp
total_score: 16
p0_count: 2
p1_count: 2
timestamp: 2026-05-28T20-25-19Z
slug: reactapp
---
## Design Health Score

| # | Heuristic | Score | Key Issue |
|---|-----------|-------|-----------|
| 1 | Visibility of System Status | 2 | Patch-banner exposes RFC 6901 paths to end users; success toast reads "Change have been saved." (typo); loader is theatrical not informative. |
| 2 | Match System / Real World | 2 | "ApplyError · /args/inlineData/data/0/x · 8a3c4d1e" surfaces engineering vocabulary to dashboard editors. |
| 3 | User Control and Freedom | 3 | Cancel/Save/Escape on inline edits are clean. Drag-reorder for tabs works. |
| 4 | Consistency and Standards | 1 | Three "primary blues" (#1e6b8b, #0a62a9, #007bff, plus #0d6efd in modals), seven icon families mixed (Bs/Fa/Fa6/Ci/Hi/Md/Io/Ai), 119 inline style={{}} + 311 styled.* + 136 react-bootstrap imports, no token layer. |
| 5 | Error Prevention | 2 | NormalInput does mid-typing number validation (good). But "Change have been saved" is shipped copy. |
| 6 | Recognition Rather Than Recall | 1 | Header is 7+ unlabeled icon chips on a saturated blue navbar. BsGrid3X3Gap means "Return to Landing Page", BsFillPersonFill means "Login", composite LockedIcon means "Lock Movement". None recognizable. |
| 7 | Flexibility and Efficiency | 1 | Zero discoverable keyboard shortcuts on a power-editor tool. Slash-prompts are three generic strings. |
| 8 | Aesthetic and Minimalist Design | 1 | Landing page is identical-card-grid. Dashboard card stacks 3 transparency layers on hover. Loader is a 300px planetary system. Header buttons use rgba(255,255,255,0.1) chips on bg="primary" with near-zero contrast. |
| 9 | Error Recovery | 1 | "Failed to delete dashboard." / "Failed to copy public link." / "Failed to save changes. Check server logs for more information." No codes, no retry, no path forward. |
| 10 | Help and Documentation | 2 | AppInfo modal + AppTour exist. No inline help, no glossary, chat has no "what can I ask" surface. |
| **Total** | | **16/40** | **Functional but visually fractured** |

## Anti-Patterns Verdict

**LLM assessment.** Hand-rolled product UI, not generated slop, but it doesn't pass the "trust it like Linear/Notion/Figma" bar. The dominant tell is design-system fracture: the codebase has accumulated three distinct visual vocabularies (Bootstrap-default, custom styled-components, inline-style one-offs) that visibly clash within the same view. Header chrome looks Tethys-2018, modals look stock Bootstrap, the loader looks like a CodePen experiment, the chat sidebar looks 2024 SaaS. Same product, four eras.

**Deterministic scan** (detect.mjs --fast): 5 hits.
- components/sidebar/ChatSidebar.js:57 — transition: width, min-width (banned layout animation).
- components/map/LayersControl.js:30 — transition: width.
- components/modals/MapLayer/SourcePane.js:107 — border-left: 3px solid #0d6efd (side-stripe accent, absolute ban).
- components/visualizations/Map.js:345 — border-top: 4px solid on rounded element.
- components/inputs/TextEditor.js:44 — border-right: 1px solid lightgray (likely separator, false positive).

**Visual overlays**: not available (JSX source review, no running browser).

## Overall Impression

Functional bones are good: context-driven state, per-tile error boundaries, per-dashboard chat history, drag-reorder tabs, promise-based delete confirmation, structured patch-rejection feedback. Someone has thought hard about correctness.

But the surface is untokenized. A user sliding their eyes across "Available Dashboards" header → a dashboard card → the editing sidebar → a modal sees four different design systems agreeing only on the Bootstrap grid. Biggest unlock is collapsing the visual vocabulary: one blue, one icon family, one form-control style, one button vocabulary, one type ramp, one spacing rhythm.

## What's Working

- Per-tile error boundary + sidebar patch-rejected banner. Failure modes are designed for, not assumed away.
- State plumbing is honest. Context API throughout, ?? {} defaults so isolated tests don't crash, key={dashboardUuid} on Chatbox to force-remount cleanly.
- NormalInput mid-typing number validation. Lets ${variable} substitution and "-.5" typing both work without flickering invalid state.

## Priority Issues

**[P0] Visual identity has fractured into 3+ design systems.**
- Why it matters: Editors hesitate at every hand-off between header chrome / dashboard grid / modals / sidebar. Adoption ceiling.
- Where: custom-bootstrap.scss declares $primary: #1e6b8b. LoadingAnimation.scss:2 declares --bs-primary: #0a62a9. DashboardCard.js:115, DashboardTabs.js:38, DashboardTabs.js:55 use #007bff. Header.js uses bg="primary". SourcePane.js:107 uses #0d6efd. Same role, five hex values.
- Fix: Build a token layer (CSS custom properties for --ts-color-primary, --ts-color-primary-hover, --ts-color-focus, --ts-color-border, --ts-color-surface, --ts-text-strong, --ts-text-muted). Replace every raw hex with the token. Pick ONE icon family. Codify what styled is for vs. inline style={{}} vs. CSS file.
- Suggested command: /impeccable document then /impeccable distill.

**[P0] The header icon strip fails first-time users and frustrates power users.**
- Why it matters: Primary action surface is 5–8 unlabeled icon chips on a saturated-blue bar with rgba(255,255,255,0.1) backgrounds that have near-zero contrast against the bar itself (HeaderButton.css:6).
- Where: BsGrid3X3Gap for "Return to Landing Page" (wrong convention). BsFillPersonFill for "Login" (it's a profile icon). Composite LockedIcon (padlock with expand-arrows overlay) for movement-lock is an invented affordance.
- Fix: Promote primary editing actions (Save, Edit, Add) to icon-with-text buttons; demote utility (Settings, Info, Logout) to a single overflow menu. Replace BsGrid3X3Gap with BsHouse / BsArrowLeft. Replace BsFillPersonFill with text "Sign in". Replace composite LockedIcon with a single BsLock / BsUnlock.
- Suggested command: /impeccable clarify then /impeccable distill.

**[P1] Error messages leak engineering language to end users.**
- Why it matters: "Patch failed: ApplyError · /args/inlineData/data/0/x · 8a3c4d1e" in ChatSidebar.js:417-421 is a tile editor's primary failure signal. "Check server logs for more information" (Header.js:463) puts ops burden on someone whose mental model is "I clicked Save."
- Fix: Map the patch error envelope to human English at the banner layer ("Couldn't edit chart 'Monthly Streamflow' — that field can't be changed from chat. Try the Edit panel."). Keep JSON-pointer in a collapsed "show details" disclosure for support. Add retry to network failures.
- Suggested command: /impeccable clarify.

**[P1] No discoverable keyboard shortcuts on a power-editor tool.**
- Why it matters: Editor's action vocabulary (Save, Add, Edit, Cancel, Open Chat, Switch Tab) is the canonical case for shortcuts. Today there are none beyond Enter / Escape on text inputs.
- Fix: Cmd+S to save in editing mode, Cmd+E to toggle editing, Cmd+K to focus chat, Cmd+/ to open the slash menu, Cmd+B to toggle sidebar, Cmd+Shift+N to add panel. Surface them as right-aligned hints in each tooltip and in a ? cheatsheet modal.
- Suggested command: /impeccable shape.

**[P2] The loader is 60% of first-paint visual budget on something users see for 200ms.**
- Why it matters: LoadingAnimation.scss builds a 300×300px planetary system (two orbital arcs, four "moons", 13s/15s spin) with "Loading TethysDash..." in 40pt. Wrong genre — product apps load into a task. Locks identity to "playful brand mark" which contradicts rest of the chrome.
- Fix: Replace with a single 32px spinner in the primary color, or a content skeleton matching the landing-page card grid. Reserve orbital animation for the actual wordmark, not the loading indicator.
- Suggested command: /impeccable quieter.

## Persona Red Flags

**Alex (Power User — hydrologist authoring her team's dashboard).** ~8 clicks plus tooltip-reads to add a panel, save, switch tabs. Zero keyboard shortcuts. Tooltip trigger={["hover", "click"]} fires on touch drag. 100ms setTimeout choreography in onEdit/onCancel (Header.js:360, 442) creates click races. Patch-rejected banner shows /args/inlineData/data/0/x and she has to translate.

**Jordan (First-time dashboard builder).** Lands on dashed "Create a New Dashboard" tile, fine. Picks an existing dashboard. Header is now 7 unlabeled icons. Cannot tell what the chat does until they open it. Welcome message says "Ask me about your dashboard" and offers "Create a bar chart" but they haven't loaded data. Bounces.

**Reza (Skeptical hydrologist, evaluating).** Reads "Change have been saved" toast within 90 seconds. Notices three competing blues. Decides this is grad-student software. Closes tab.

## Minor Observations

- DashboardCard.js:46-67 stacks three transparency layers on hover (image opacity → 0.5, body background rgba(169,169,169,0.5), description overlay rgba(0,0,0,0.5)). Read together they vibrate.
- Header.js:458: setSuccessMessage("Change have been saved.") — typo, shipped copy.
- Header.js:64-75: WhiteTitle is position: absolute; left: 50%; white-space: nowrap. No truncation rule; long dashboard name collides with action icons.
- ChatSidebar.js:57: transition: width, min-width 0.3s — confirmed banned layout animation. Use transform: translateX with off-canvas pre-render, or grid-template-columns.
- App.scss:30-50 custom scrollbar #888 on #f1f1f1 — disconnected from brand color, -webkit-only.
- TooltipButton.js:33 trigger={["hover", "click"]} causes double-tap pattern on touch.
- Header.js:336-348 duplicates the useEffect from lines 305-317 verbatim. Dead duplicate.
- Modal-by-default architecture: 9 modals total. DashboardEditor would read better as an inline panel.
- BsXLg size={10} for patch-rejected dismiss (ChatSidebar.js:427) — 10px hit target, fails WCAG 2.5.8.

## Questions to Consider

- What if the header bar were one icon-with-label primary action (Edit / Save) plus an overflow menu, instead of seven peer icons?
- Does the dashboard description need to be hidden until hover, or could it sit in a permanent footer strip on each card?
- What would the landing page look like if cards weren't all 20rem × 15rem? A wider hero for most-recently-opened, a tighter list for the long tail?
- Does the chatbox need to live in a 360px right rail, or could it slide over as a command palette so the dashboard stays full-width when not in use?
- What would a confident version of the loader look like? (Hint: probably no orbital arcs.)
114 changes: 114 additions & 0 deletions .impeccable/critique/2026-05-29T14-25-24Z__reactapp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
---
target: reactapp/
total_score: 23
p0_count: 3
p1_count: 2
timestamp: 2026-05-29T14-25-24Z
slug: reactapp
---
## Design Health Score

| # | Heuristic | Score | Key Issue |
|---|-----------|-------|-----------|
| 1 | Visibility of System Status | 3 | PR #134 added real edit-lock visibility via disabled-state + tooltip; chatbox lacks a turn indicator visible outside the menu. |
| 2 | Match System / Real World | 3 | "Editing disabled while dashboard is updating" is plain English. `[in-turn delta]` JSON tokens could leak in error envelopes. |
| 3 | User Control and Freedom | 2 | `editGridItem` short-circuits with `if (isStreaming) return;` at `DashboardItem.js:382` — silent no-op, no toast. No Escape-to-cancel streaming surfaced. |
| 4 | Consistency and Standards | 1 | Drift Kill violated on brand-truth file: `LoadingAnimation.scss:2` sets `--bs-primary: #0a62a9` overriding `custom-bootstrap.scss:2`. Twelve icon families coexist. Three different "primary blues" visible per session. |
| 5 | Error Prevention | 3 | `confirm()` on delete, PR #134 defense-in-depth on streaming, patch-rejected banner. Drag-to-reorder unguarded during streaming. |
| 6 | Recognition Rather Than Recall | 3 | Tiles have title bars + dropdowns. Chatbox tool naming hidden behind `[in-turn delta]` JSON. |
| 7 | Flexibility and Efficiency | 2 | Zero keyboard shortcuts in modified surfaces. Order submenu is hover-only flyout (`DashboardItemDropdown.js:123-135`). |
| 8 | Aesthetic and Minimalist Design | 1 | `LoadingAnimation.js:18-39` renders 24 DOM elements (2 orbits × 4 arcs + 2 moons + 40pt label) per dashboard load — consumer-fitness theatrical loader DESIGN.md anti-reference #4. |
| 9 | Error Recovery | 3 | `patchRejections` FIFO cap=5, per-tile ErrorBoundary, structured patch envelope. Strong. |
| 10 | Help and Documentation | 2 | `welcomeHeading` + 3 suggested prompts. No shortcut hint, no command reference. |
| **Total** | | **23 / 40** | **Improved from 2026-05-28's 16/40 on behavioral discipline (PR #134); visual/aesthetic drift unchanged.** |

## Anti-Patterns Verdict

**LLM assessment.** Hand-rolled but visibly fractured. PR #134 is technically careful (StreamingContext partition justified by re-render scope, semantic `disabled` props, defense-in-depth). But it lives inside a system that violates DESIGN.md in concrete, traceable ways. The 2026-05-28 critique named `SourcePane.js:107`'s side-stripe — it survived PRs #134 + #135 unchanged. That's the strongest signal: DESIGN.md is not a merge gate.

**Deterministic scan** (`detect.mjs --fast`): exit 2, 5 findings.

- `side-tab` — `modals/MapLayer/SourcePane.js:107` (`border-left: 3px solid #0d6efd`): real, DESIGN.md ban + Drift Kill + critique repeat.
- `side-tab` — `inputs/TextEditor.js:44` (`border-right: 1px solid lightgray`): likely FP (column separator).
- `layout-transition` — `sidebar/ChatSidebar.js:57` (`transition: width, min-width`): real, motion rule.
- `layout-transition` — `map/LayersControl.js:30` (`transition: width`): real, motion rule.
- `border-accent-on-rounded` — `visualizations/Map.js:345` (`border-top: 4px solid`): real, side-stripe family.

**Visual overlays**: not available (no dev server this session; CLI scan only).

## What's Working

- **StreamingContext partition** (`contexts/Contexts.js:16-28`). New context instead of overloading existing context. Comment justifies the re-render-scope decision.
- **Defense-in-depth gating in PR #134** (`DashboardItem.js:366, 382, 397` + `DashboardItemDropdown.js`). Handler guards AND visual disabled-state.
- **Per-dashboard chat history with key remount** (`ChatSidebar.js:438`). Clean state reset across dashboard switches.

## Priority Issues

**[P0] Drift hex on the brand-truth file**

- **What**: `LoadingAnimation.scss:2` sets `--bs-primary: #0a62a9` in `html { }` scope, overriding `custom-bootstrap.scss:2`'s canonical `$primary: #1e6b8b`. Three more drift values scattered (`#007bff`, `#0d6efd`, `#0a62a9`). All on DESIGN.md's Drift Kill list.
- **Why it matters**: viewers see a non-canonical primary for the first ~500ms of every navigation. The brand-truth file STILL CONTAINS the prohibited value — loudest "no one is treating DESIGN.md as a gate" signal.
- **Fix**: delete the `html { --bs-primary: ... }` block. Codemod the drift hexes to `var(--bs-primary)` or `watershed-slate` token references.
- **Suggested command**: `/impeccable harden`

**[P0] Theatrical planetary loader on every dashboard navigation**

- **What**: `LoadingAnimation.js:18-39` renders 24 DOM elements per dashboard load. Consumer-fitness-app theatrical loader; DESIGN.md anti-reference #4 by name.
- **Why it matters**: PRODUCT.md's first-impression contract is "my dashboards are right there, ready to open." Maya sees this 50×/day; Sam reads it as "brochure-ware logo experience"; Lin projects it to stakeholders during a briefing.
- **Fix**: replace with a 16px top progress bar OR a single 24px spinner in `#1e6b8b`. Drop the 40pt text to body size. Skip entirely on `prefers-reduced-motion: reduce`.
- **Suggested command**: `/impeccable quieter`

**[P0] Side-stripe border-left accent still present**

- **What**: `modals/MapLayer/SourcePane.js:107` — `border-left: 3px solid #0d6efd;` Detector flagged it; 2026-05-28 critique flagged it; survived PRs #134/#135. Plus `visualizations/Map.js:345` `border-top: 4px solid` (same family). Plus `DashboardTabs.js:103` 1px border-left on active tab (borderline by spec threshold, visually same pattern).
- **Why it matters**: DESIGN.md absolute ban + named in prior critique. Two PRs landed without anyone looking — strongest evidence DESIGN.md isn't a merge gate today.
- **Fix**: replace GeoTIFF hint with flat token-background card (`bg-secondary`, 4px radius, no stripe). Replace `border-top: 4px` on Map.js:345 with nothing or inset shadow. Replace active-tab `border-left` with `box-shadow: inset 0 -2px 0 var(--bs-primary)`.
- **Suggested command**: `/impeccable distill`

**[P1] Silent no-op on edit-during-streaming**

- **What**: `DashboardItem.js:382` — `if (isStreaming) return;` short-circuits the open-modal path. Dropdown is disabled with tooltip; double-click or other paths get no feedback.
- **Why it matters**: Heuristic #1 violation. PR #134 added `STREAMING_DISABLED_TITLE` tooltip but didn't propagate the signal to the open-modal path. Sam will report a bug.
- **Fix**: dispatch a brief `CustomAlert` ("Editing paused — dashboard is updating") via the existing alert system at `DashboardItem.js:540-551`.
- **Suggested command**: `/impeccable clarify`

**[P1] Twelve icon families coexist**

- **What**: `Bs Fa Fa6 Bi Cg Ci Hi Lu Md Ri Rx Tb` all imported across `reactapp/components/`. DESIGN.md: one family. `DashboardCard.js` alone imports from `Bs` + `Fa`. The seven the prior critique counted has grown to twelve.
- **Why it matters**: most visible "different person built each panel" signal. Bs (rounded) and Fa (sharper, marker-style) sit next to each other on every dashboard card header.
- **Fix**: pick `Bs` (dominant family already, aligned with Bootstrap in stack). Codemod the eleven other families' imports to `Bs*` equivalents. Add ESLint rule disallowing import from other families.
- **Suggested command**: `/impeccable distill`

## Persona Red Flags

**Maya (water-resource scientist, daily editor)**:
- 40pt "Loading Dashboard..." planetary spinner 50+ times/week (`LoadingAnimation.js:38`). Un-serious for a workday tool.
- Double-clicks Edit mid-chatbox-turn → silent no-op (`DashboardItem.js:382`). She double-clicks, doesn't open menu first. Hidden state.
- Three different blues in one session: loader `#0a62a9`, info icon `#007bff`, GeoTIFF hint `#0d6efd`. Reads as inconsistency.

**Sam (workshop participant, first-time user)**:
- Planetary spinner on first load → "brochure-ware logo experience" as first impression.
- Streams a tile → tries to Edit → nothing happens → looks for an error → will report a bug.
- Hover-only Order submenu fiddly on trackpad. He won't discover Bring-to-Front exists.

**Lin (dashboard viewer, no edit rights)**:
- `editable = false` → ChatSidebar correctly returns null. Good.
- Planetary loader fires on every navigation regardless of permission. Projects to room of stakeholders.
- Owner Icon + Public Icon irrelevant chrome for viewer-only briefing context.

## Minor Observations

- `DashboardItem.js:103-104` — `rgba(0,0,0,0.97)` + `#ffffffff`. Trace Rule violations.
- `ChatSidebar.js:62` — `background: #fff`. Trace Rule violation.
- `ChatSidebar.js:106-107` — Patch-failed banner `#fff8e1` on `#f0d68a`. Government-portal yellow.
- `ChatSidebar.js:457` — em-dash + "just ask" — em-dash banned by shared design laws, "just ask" reads fitness-warm.
- `LoadingAnimation.js:13` — `setTimeout` with no cleanup.
- `DashboardItemDropdown.js:38` — `STREAMING_DISABLED_TITLE` hard-coded English (i18n flag).

## Provocative Questions

1. Why does TethysDash need a loader animation at all?
2. Should streaming state be invisible when the user isn't actively trying to edit?
3. Is the per-tile dropdown the right vocabulary, vs. Linear-style keyboard focus + E/D/Cmd+]?
4. Why is the GeoTIFF hint a side-stripe panel — alert or popover, but not stripe?
5. Could the four blues collapse to one?
Loading