UX-24: Inbox color-coded status tags and text fatigue reduction#631
UX-24: Inbox color-coded status tags and text fatigue reduction#631Chris0Jeky merged 3 commits intomainfrom
Conversation
- Status chips now use semantic colors: Failed (red), Applied to Board (green), Triaging/Triaged/Ready for review (amber), New (ember), Ignored (muted gray unchanged) - Excerpt truncated to 2 lines with line-clamp and font-weight 400 - Alternating row backgrounds for visual rhythm Closes #624
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces status-based color coding for chips in the inbox view, adds alternating row backgrounds for better readability, and implements line-clamping for task excerpts. Feedback focuses on correcting the CSS selector for alternating backgrounds in the virtual list, reducing logic duplication between status labels and classes, and ensuring consistent line-clamping across mobile and desktop views.
| .td-inbox-row:nth-child(even) { | ||
| background: var(--td-surface-container-low, #1e1d1d); | ||
| } |
There was a problem hiding this comment.
The :nth-child(even) selector on .td-inbox-row will not work as intended. In the template (lines 723-731), each .td-inbox-row is wrapped inside a div created by the v-for loop. Consequently, every .td-inbox-row is the first and only child of its immediate parent. To achieve alternating backgrounds in this virtual list structure, you should target the sibling wrapper elements instead.
.td-inbox__list div[role="presentation"] > div:nth-child(even) .td-inbox-row {
background: var(--td-surface-container-low, #1e1d1d);
}
| function statusChipClass(status: CaptureStatusValue): string { | ||
| if (status === 0 || status === 'New') return 'td-status-chip--new' | ||
| if (status === 1 || status === 'Triaging') return 'td-status-chip--triaging' | ||
| if (status === 2 || status === 'Triaged') return 'td-status-chip--triaging' | ||
| if (status === 3 || status === 'ProposalCreated') return 'td-status-chip--triaging' | ||
| if (status === 4 || status === 'Converted') return 'td-status-chip--converted' | ||
| if (status === 5 || status === 'Ignored') return 'td-status-chip--ignored' | ||
| if (status === 6 || status === 'Failed') return 'td-status-chip--failed' | ||
| return '' | ||
| } |
There was a problem hiding this comment.
The logic in statusChipClass heavily duplicates the conditional structure and mapping found in statusLabel (lines 160-169). This increases maintenance overhead, as any future status additions or changes will require updates in both functions. Consider refactoring these into a shared mapping object or a unified helper function that returns both the label and the CSS class for a given status.
| font-weight: 400; | ||
| line-height: 1.5; | ||
| display: -webkit-box; | ||
| -webkit-line-clamp: 2; |
There was a problem hiding this comment.
There is a discrepancy between the desktop and mobile line-clamping for excerpts. While this change sets a 2-line limit for desktop, the existing mobile media query (line 1389) still uses -webkit-line-clamp: 3. To align with the PR's goal of "text fatigue reduction," mobile should likely be restricted to 2 lines (or even 1) to save vertical space on smaller screens.
nth-child(even) does not work with virtualized rows since each td-inbox-row is the sole child of its wrapper div. Use index-based class (td-inbox-row--alt) instead to ensure alternating backgrounds render correctly regardless of virtual DOM structure.
Adversarial Self-ReviewIssues found and fixed
Potential concerns reviewed (no issues found)
|
Match desktop truncation for consistent text fatigue reduction.
1 participant
Summary
line-clampwithfont-weight: 400to reduce visual densitynth-child(even)) for improved scanabilityTest plan
npm run typecheckpassesnpm run buildpassesCloses #624