-
Notifications
You must be signed in to change notification settings - Fork 127
Port upstream CodexBar 0.56.1 [review] #435
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.
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
Changes from all commits
9153ebc
386796f
7661ddb
77f5225
9b77711
0b29d5a
38ddcb2
d71cf51
e120383
3f17392
8111ec2
78239c8
6054e91
f9fe35d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import { render } from "@testing-library/react"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import { BarChart } from "./BarChart"; | ||
|
|
||
| describe("BarChart calendar slots", () => { | ||
| it("keeps unknown and known-zero slots distinct", () => { | ||
| const { container } = render( | ||
| <BarChart | ||
| data={[ | ||
| { label: "unknown", value: null }, | ||
| { label: "zero", value: 0 }, | ||
| { label: "known", value: 2 }, | ||
| ]} | ||
| ariaLabel="history" | ||
| animations={false} | ||
| />, | ||
| ); | ||
| const bars = container.querySelectorAll(".chart__bar"); | ||
| expect(bars).toHaveLength(3); | ||
| expect(bars[0]).toHaveAttribute("opacity", "0"); | ||
| expect(bars[1]).toHaveAttribute("opacity", "0.25"); | ||
| expect(container).toHaveTextContent("unknown"); | ||
| expect(container).toHaveTextContent("zero: 0.00"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ import { useChartAnimation } from "./useChartAnimation"; | |
|
|
||
| export interface BarChartPoint { | ||
| label: string; | ||
| value: number; | ||
| value: number | null; | ||
| } | ||
|
|
||
| export interface BarChartProps { | ||
|
|
@@ -59,7 +59,7 @@ export function BarChart({ | |
| let p = -1; | ||
| for (let i = 0; i < data.length; i++) { | ||
| const v = data[i].value; | ||
| if (v > m) { | ||
| if (v != null && v > m) { | ||
| m = v; | ||
| p = i; | ||
| } | ||
|
|
@@ -101,7 +101,7 @@ export function BarChart({ | |
| aria-label={ariaLabel} | ||
| > | ||
| {data.map((p, i) => { | ||
| const base = p.value === 0 ? 1 : Math.max(3, (p.value / max) * plotHeight); | ||
| const base = p.value == null ? 1 : p.value === 0 ? 1 : Math.max(3, (p.value / max) * plotHeight); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Run Windows build and CUA validation for all changed chart surfaces. On Windows, run 🤖 Prompt for AI Agents |
||
| const eased = anim.barProgress(i); | ||
| const barH = base * eased; | ||
| const x = i * (barWidth + BAR_GAP); | ||
|
|
@@ -119,14 +119,14 @@ export function BarChart({ | |
| width={barWidth} | ||
| height={bodyH} | ||
| fill={color} | ||
| opacity={p.value === 0 ? 0.25 : isHovered ? 1 : 0.9} | ||
| opacity={p.value == null ? 0 : p.value === 0 ? 0.25 : isHovered ? 1 : 0.9} | ||
| rx={1} | ||
| className="chart__bar" | ||
| onMouseMove={(e) => onMove(e, i)} | ||
| onMouseMove={p.value == null ? undefined : (e) => onMove(e, i)} | ||
| onMouseLeave={onLeave} | ||
| > | ||
| <title> | ||
| {p.label}: {fmt(p.value)} | ||
| {p.value == null ? p.label : `${p.label}: ${fmt(p.value)}`} | ||
| </title> | ||
| </rect> | ||
| {isPeak && ( | ||
|
|
@@ -157,7 +157,7 @@ export function BarChart({ | |
| role="tooltip" | ||
| > | ||
| <span className="chart__tooltip-label">{data[hover.i].label}</span> | ||
| <strong>{fmt(data[hover.i].value)}</strong> | ||
| <strong>{fmt(data[hover.i].value ?? 0)}</strong> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Hide the tooltip when the hovered value is unknown. When refreshed 🤖 Prompt for AI Agents |
||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { render } from "@testing-library/react"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import { LineChart } from "./LineChart"; | ||
|
|
||
| describe("LineChart unknown values", () => { | ||
| it("renders gaps for unknown values while preserving known zero", () => { | ||
| const { container } = render( | ||
| <LineChart | ||
| data={[ | ||
| { label: "2026-09-01", value: 1 }, | ||
| { label: "2026-09-02", value: 0 }, | ||
| { label: "2026-09-03", value: null }, | ||
| { label: "2026-09-04", value: 2 }, | ||
| { label: "2026-09-05", value: 3 }, | ||
| ]} | ||
| ariaLabel="credits history" | ||
| animations={false} | ||
| />, | ||
| ); | ||
|
|
||
| expect(container.querySelectorAll(".chart__point")).toHaveLength(4); | ||
| expect(container.querySelectorAll(".chart__line")).toHaveLength(2); | ||
| expect(container).toHaveTextContent("2026-09-02: 0.00"); | ||
| expect(container).not.toHaveTextContent("2026-09-03: 0.00"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: nesszer/Win-CodexBar
Length of output: 39865
🏁 Script executed:
Repository: nesszer/Win-CodexBar
Length of output: 13077
🏁 Script executed:
Repository: nesszer/Win-CodexBar
Length of output: 50376
🏁 Script executed:
Repository: nesszer/Win-CodexBar
Length of output: 46756
Derive coverage from the persisted scan range.
parse_codex_filecan add a file with emptydays.rebuild_cache_daysthen leavescache.daysempty, while the completed scan persistsscan_since_key,scan_until_key, and noprevious_report. The debounce branch accepts this cache throughcache.files, butcached_history_coverage_establishedchecks!cache.days.is_empty().usage_spend.rstherefore marks the completed zero-usage result asrefreshinginstead ofknown_zero.Use the covered scan range and completion metadata to derive coverage. Add a regression test for a cache with files, empty
days, a complete range, and noprevious_report.🤖 Prompt for AI Agents