Skip to content

Commit 96a07b6

Browse files
Aurelioloclaude
Aureliolo
and
claude
committed
fix: address review findings from pre-PR agents
- Fix division-by-zero in Sparkline when data has 1 element - Fix division-by-zero in MetricCard when progress.total is 0 - Add animated draw to Sparkline (stroke-dasharray + fade-in) - Replace hardcoded shadow in AgentCard with --so-shadow-card-hover token - Add aria-label to MetricCard progressbar - Replace hardcoded text-white with text-foreground in SidebarNavItem - Add borderColor prop to Avatar for department-colored borders - Add .density-medium level to design-tokens.css - Add bg-card-hover to brand-and-ux.md color token table Pre-reviewed by 3 agents, 14 findings addressed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0153827 commit 96a07b6

9 files changed

Lines changed: 66 additions & 8 deletions

File tree

‎docs/design/brand-and-ux.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ Colors are **state-driven**, not decorative. Every colored element answers: "wha
3434
| `bg-base` | Page background | `#0a0a12` | Deepest background layer |
3535
| `bg-surface` | Sidebar, elevated surfaces | `#0f0f1a` | Sidebar, panels, raised areas |
3636
| `bg-card` | Card backgrounds | `#13131f` | All card containers |
37+
| `bg-card-hover` | Card hover state | `#181828` | Card background on mouse-over |
3738
| `border` | Default borders | `#1e1e2e` | Card borders, dividers |
3839
| `border-bright` | Interactive/hover borders | `#2a2a3e` | Focus rings, hover states |
3940

‎web/src/__tests__/components/ui/sparkline.test.tsx‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ describe('Sparkline', () => {
2828
it('renders nothing for single data point', () => {
2929
const { container } = render(<Sparkline data={[5]} />)
3030

31-
// A single point can't form a line, still renders the dot
32-
const svg = container.querySelector('svg')
33-
expect(svg).toBeInTheDocument()
31+
expect(container.querySelector('svg')).not.toBeInTheDocument()
3432
})
3533

3634
it('renders a polyline element', () => {

‎web/src/components/layout/SidebarNavItem.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export function SidebarNavItem({
4444
className={cn(
4545
'flex size-5 items-center justify-center',
4646
'rounded-full bg-danger',
47-
'text-xs font-semibold text-white',
47+
'text-xs font-semibold text-foreground',
4848
)}
4949
>
5050
{badge > 99 ? '99+' : badge}

‎web/src/components/ui/agent-card.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function AgentCard({
2626
className={cn(
2727
'rounded-lg border border-border bg-card p-card',
2828
'transition-all duration-200',
29-
'hover:bg-card-hover hover:-translate-y-px hover:shadow-[0_4px_24px_rgba(56,189,248,0.08)]',
29+
'hover:bg-card-hover hover:-translate-y-px hover:shadow-[var(--so-shadow-card-hover)]',
3030
className,
3131
)}
3232
>

‎web/src/components/ui/avatar.stories.tsx‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ export const Large: Story = {
3636
args: { name: 'Alice Smith', size: 'lg' },
3737
}
3838

39+
export const WithBorder: Story = {
40+
args: { name: 'Alice Smith', borderColor: 'border-accent' },
41+
}
42+
3943
export const AllSizes: Story = {
4044
args: { name: 'Alice Smith' },
4145
render: () => (
@@ -46,3 +50,15 @@ export const AllSizes: Story = {
4650
</div>
4751
),
4852
}
53+
54+
export const WithDepartmentBorders: Story = {
55+
args: { name: 'Alice Smith' },
56+
render: () => (
57+
<div className="flex items-center gap-3">
58+
<Avatar name="Alice Smith" borderColor="border-accent" />
59+
<Avatar name="Bob Jones" borderColor="border-success" />
60+
<Avatar name="Carol White" borderColor="border-warning" />
61+
<Avatar name="Dave Brown" borderColor="border-danger" />
62+
</div>
63+
),
64+
}

‎web/src/components/ui/avatar.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ function getInitials(name: string): string {
1818
interface AvatarProps {
1919
name: string
2020
size?: 'sm' | 'md' | 'lg'
21+
borderColor?: string
2122
className?: string
2223
}
2324

24-
export function Avatar({ name, size = 'md', className }: AvatarProps) {
25+
export function Avatar({ name, size = 'md', borderColor, className }: AvatarProps) {
2526
const initials = getInitials(name)
2627

2728
return (
@@ -31,6 +32,8 @@ export function Avatar({ name, size = 'md', className }: AvatarProps) {
3132
className={cn(
3233
'inline-flex shrink-0 items-center justify-center rounded-full',
3334
'bg-accent-dim font-mono font-semibold text-foreground',
35+
borderColor && 'border-2',
36+
borderColor,
3437
SIZE_CLASSES[size],
3538
className,
3639
)}

‎web/src/components/ui/metric-card.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export function MetricCard({
2020
subText,
2121
className,
2222
}: MetricCardProps) {
23-
const progressPct = progress
23+
const progressPct = progress && progress.total > 0
2424
? Math.round((progress.current / progress.total) * 100)
2525
: 0
2626

@@ -55,6 +55,7 @@ export function MetricCard({
5555
aria-valuenow={progressPct}
5656
aria-valuemin={0}
5757
aria-valuemax={100}
58+
aria-label={`${label} progress`}
5859
className="mt-2 h-0.5 w-full overflow-hidden rounded-full bg-border"
5960
>
6061
<div

‎web/src/components/ui/sparkline.tsx‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ interface SparklineProps {
66
color?: string
77
width?: number
88
height?: number
9+
animated?: boolean
910
className?: string
1011
}
1112

@@ -29,11 +30,12 @@ export function Sparkline({
2930
color = 'var(--so-accent)',
3031
width = 64,
3132
height = 24,
33+
animated = true,
3234
className,
3335
}: SparklineProps) {
3436
const gradientId = useId()
3537

36-
if (data.length === 0) return null
38+
if (data.length <= 1) return null
3739

3840
const points = buildPoints(data, width, height)
3941
const pointPairs = points.split(' ')
@@ -46,6 +48,9 @@ export function Sparkline({
4648
const padding = 2
4749
const fillPoints = `${padding},${height - padding} ${points} ${width - padding},${height - padding}`
4850

51+
// Approximate total path length for draw animation
52+
const approxPathLength = width * 1.5
53+
4954
return (
5055
<svg
5156
width={width}
@@ -55,6 +60,24 @@ export function Sparkline({
5560
className={cn('shrink-0', className)}
5661
aria-hidden="true"
5762
>
63+
{animated && (
64+
<style>{`
65+
@keyframes sparkline-draw {
66+
from { stroke-dashoffset: ${approxPathLength}; }
67+
to { stroke-dashoffset: 0; }
68+
}
69+
@keyframes sparkline-fade {
70+
from { opacity: 0; }
71+
to { opacity: 1; }
72+
}
73+
@media (prefers-reduced-motion: reduce) {
74+
.sparkline-line, .sparkline-fill, .sparkline-dot {
75+
animation: none !important;
76+
}
77+
}
78+
`}</style>
79+
)}
80+
5881
<defs>
5982
<linearGradient id={gradientId} x1="0" y1="0" x2="0" y2="1">
6083
<stop offset="0%" stopColor={color} stopOpacity="0.3" />
@@ -64,26 +87,36 @@ export function Sparkline({
6487

6588
{/* Fill area */}
6689
<polygon
90+
className="sparkline-fill"
6791
points={fillPoints}
6892
fill={`url(#${gradientId})`}
93+
style={animated ? { animation: 'sparkline-fade 200ms ease-out 200ms both' } : undefined}
6994
/>
7095

7196
{/* Line */}
7297
<polyline
98+
className="sparkline-line"
7399
points={points}
74100
stroke={color}
75101
strokeWidth="1.5"
76102
strokeLinecap="round"
77103
strokeLinejoin="round"
78104
fill="none"
105+
style={animated ? {
106+
strokeDasharray: approxPathLength,
107+
strokeDashoffset: 0,
108+
animation: `sparkline-draw 200ms ease-out 200ms both`,
109+
} : undefined}
79110
/>
80111

81112
{/* End dot */}
82113
<circle
114+
className="sparkline-dot"
83115
cx={lastX}
84116
cy={lastY}
85117
r="2"
86118
fill={color}
119+
style={animated ? { animation: 'sparkline-fade 200ms ease-out 400ms both' } : undefined}
87120
/>
88121
</svg>
89122
)

‎web/src/styles/design-tokens.css‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@
9393
--so-density-grid-gap: var(--so-space-3);
9494
}
9595

96+
.density-medium {
97+
--so-density-card-padding: 14px;
98+
--so-density-section-gap: var(--so-space-4);
99+
--so-density-grid-gap: var(--so-space-4);
100+
}
101+
96102
.density-sparse {
97103
--so-density-card-padding: var(--so-space-5);
98104
--so-density-section-gap: var(--so-space-6);

0 commit comments

Comments
 (0)