Skip to content

fix(charts): draw dumbbell gridlines and tighter rows so the change reads at a glance - #97

Open
onmax wants to merge 16 commits into
mainfrom
fix/charts-dumbbell
Open

onmax wants to merge 16 commits into
mainfrom
fix/charts-dumbbell

Conversation

@onmax

@onmax onmax commented Oct 22, 2026

Copy link
Copy Markdown
Owner

The dumbbell chart was hard to read: four rows far apart, a thin pale connector, no gridlines, and an axis running well past the largest value.

  • A light gridline at every x tick, and a faint guide along each row
  • Rows 36px apart instead of 93px
  • A thicker, darker connector and bigger dots
  • The axis ends at a round tick at or just past the largest value, and doesn't have to start at 0
  • Each row prints its lowest and highest value beside its dots
  • Fixes dumbbell rows over dates, which drew one curve through every row instead of one connector per row

Only dumbbells change. The new tests in packages/charts/test/charts.test.ts check the ticks, the gridlines, the printed values, and one connector per row over dates.

🤖 Generated with Claude Code

…eads at a glance

Dumbbells now grid every tick and guide every row, pack rows 36px apart, draw a heavier connector and bigger dots, end the axis at a round tick near the data, and print each row's lowest and highest value beside its outer dots. Rows over dates are names, so each gets its own connector instead of one line through them all.
@vitehub-bot

vitehub-bot Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor
Session Status Started Duration
Pending Waiting Not started Paused

Next step

Pass interrupted; current webhook state retained.

Final answers

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

Session

Pass interrupted; current webhook state retained.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ f764935 2026-10-23T06:38:25.640Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 420c5c1 2026-10-23T06:12:42.209Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 6db5408 2026-10-23T04:27:36.912Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ bf03c1b 2026-10-23T02:19:45.140Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 1eaafc8 2026-10-23T00:03:45.800Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 5c9a7e8 2026-10-22T21:57:57.839Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 1342277 2026-10-22T19:52:31.405Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 7e45f1d 2026-10-22T17:42:44.135Z Visit the dashboard ↗
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 00a2a73 2026-10-22T15:39:20.102Z Visit the dashboard ↗
  • Build: In progress 🔵

View logs ↗
95b650c 2026-10-22T15:20:47.948Z View logs ↗

View all previews: View all previews ↗

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new dumbbell endpoint labels are computed from the wrong Vega fields after the chart's x/y encoding is flipped.

Reviewed changes

This review covers the dumbbell rendering changes and their focused HTML rendering tests.

  • Dumbbell layout — Adds explicit round x ticks, x/y gridlines, tighter rows, stronger connectors, larger dots, and endpoint text layers.
  • Date rows — Forces dumbbell rows to remain separate when the x labels are dates.
  • Coverage — Adds assertions for ticks, gridlines, endpoint values, and per-row connectors.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-5.6-sol | 𝕏

Comment thread packages/charts/src/draw.ts Outdated
@vitehub-bot

vitehub-bot Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Fixed the dumbbell label transform to aggregate numeric endpoints by row (the flipped encoding fields), and added scale room for both low and high endpoint labels so worst-case charts stay within the plot.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new label-field fix is addressed, but the incremental scale-padding changes still leave two edge cases that can produce clipped or unusable dumbbells.

Reviewed changes

This review covers the changes since the prior Pullfrog review, especially endpoint-label scale padding and the corrected per-row label aggregation.

  • Expanded endpoint padding — Added right-edge padding for the largest endpoint and updated the existing left-edge padding comment.
  • Corrected label fields — Changed the label transforms to aggregate numeric endpoints by row and render the endpoint field.
  • Retained dumbbell coverage — Kept the grid, tick, endpoint-label, and date-row connector assertions while the new edge cases remain untested.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-5.6-sol | 𝕏

Comment thread packages/charts/src/draw.ts Outdated
Comment thread packages/charts/src/draw.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The equal-endpoint fallback now avoids a degenerate scale, but the axis still receives the original duplicate tick values.

Reviewed changes

This incremental review covers the dumbbell scale guards added since the prior Pullfrog review.

  • Filtered invalid endpoints — Ignores non-finite values when deriving the dumbbell domain and skips scale adjustment when no finite endpoint remains.
  • Widened equal domains — Expands a single-value domain so identical endpoints can render on a nonzero quantitative scale.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-5.6-sol | 𝕏

Comment thread packages/charts/src/draw.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This incremental review covers the equal-endpoint scale fix and the merge resolution since the prior Pullfrog review.

  • Regenerated equal-domain ticks — Recomputed explicit axis tick values after widening an all-equal endpoint domain, removing the previously reported duplicate-tick case.
  • Preserved dumbbell rendering — Retained finite endpoint filtering, iterative label padding, corrected per-row endpoint aggregation, and the date-row connector fix.
  • Verification — pnpm --filter comark-charts test passes.

Pullfrog  | View workflow run | Using gpt-5.6-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The latest rendering refactor removes the exported drawSpec path, so every Vega-Lite chart now fails during parsing and falls back to showing only its table.

Reviewed changes

This incremental review covers the commits since 95b650c, including the rendering-helper cleanup and the final source-formatting change.

  • Inlined chart rendering — Moved the ordinary chart View construction and SVG cleanup directly into draw.
  • Simplified waterfall iteration — Removed the unused waterfall row binding from the level calculation.
  • Retained dumbbell behavior — Preserved the dumbbell gridlines, scale handling, labels, and date-row connector coverage.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-5.6-sol | 𝕏

Comment thread packages/charts/src/draw.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This incremental review covers the changes since the prior Pullfrog review, especially the restored Vega-Lite spec rendering path.

  • Restored spec rendering — Reintroduced the exported drawSpec implementation while retaining the shared renderer, sealed loader, and recursive nested-data sanitization.
  • Preserved dumbbell behavior — Kept the dumbbell gridlines, scale handling, endpoint labels, and per-date-row connector behavior unchanged.
  • Verification — pnpm --filter comark-charts test passes.

Pullfrog  | View workflow run | Using gpt-5.6-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This incremental review covers the merge commit since the prior Pullfrog review, including the landing canvas first-paint fix and its end-to-end coverage.

  • Measured canvas activation — Synchronously captured the viewport dimensions before the first canvas draw during client navigation, preventing a zero-width centering offset.
  • Added navigation coverage — Verified that the first card remains centered across animation frames after navigating to the landing page.
  • Updated generated footer credit — Removed the Comark team attribution from the generated footer.

Pullfrog  | View workflow run | Using gpt-5.6-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This incremental review covers the field-guard adjustment in packages/charts/src/draw.ts since the prior Pullfrog review.

  • Loosened field guards — Changed the dumbbell encoding checks to use field-property presence while preserving the assembled string fields used by valid package dumbbells.
  • Preserved chart behavior — Verified that the new commit does not alter the numeric endpoint, row grouping, label, or Vega-Lite spec rendering paths.

Pullfrog  | View workflow run | Using gpt-5.6-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This incremental review covers the field-type narrowing and direct field access added since the prior Pullfrog review.

  • Narrowed dumbbell fields — Required the x encoding field to be a string and safely narrowed the optional y row field before using the generated Vega-Lite fields.
  • Preserved chart behavior — Verified that valid repository dumbbells still assemble string fields, including date-labeled rows, and that the endpoint aggregation continues to use the numeric field.
  • Verification — pnpm --filter comark-charts test passes.

Pullfrog  | View workflow run | Using gpt-5.6-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This incremental review covers the final dumbbell field-guard cleanup since the prior Pullfrog review.

  • Removed redundant runtime checks — Replaced the remaining string checks with casts for the internally generated Vega-Lite field names.
  • Preserved dumbbell rendering — Confirmed the public parser still supplies string fields and the endpoint aggregation, row grouping, labels, and per-row connectors remain valid.
  • Verification — pnpm --filter comark-charts test passes.

Pullfrog  | View workflow run | Using gpt-5.6-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The latest clearance adjustment can leave valid narrow dumbbell endpoint labels at the plot edge and clip them.

Reviewed changes

This incremental review covers the clearance increase since the prior Pullfrog review.

  • Increased endpoint clearance — Raised the estimated outside-label allowance from 10px to 24px for both low and high endpoints.
  • Preserved dumbbell behavior — Retained the existing scale, gridline, row-layout, label, and per-date-row connector implementation.
  • Verification — pnpm --filter comark-charts test passes.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-5.6-sol | 𝕏

Comment thread packages/charts/src/draw.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new cap can make endpoint-padding passes expand a dumbbell's domain instead of converging for narrow charts with wide numeric labels.

Reviewed changes

This incremental review covers the endpoint-padding change since the prior Pullfrog review.

  • Capped label clearance — Replaced the need < 1 bypass with a 0.9 cap for both low and high endpoint labels.
  • Retained chart coverage — Preserved the dumbbell row layout, gridlines, labels, scale handling, and existing rendering tests.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-5.6-sol | 𝕏

Comment thread packages/charts/src/draw.ts Outdated
…cannot fit

Rows whose series tie matched the min filter once per series, so the same
value was drawn twice on one dot. The edge padding was also solved with a
stale opposite edge for three passes, which under-padded wide plots and, on
plots narrowed by long row names, pushed the scale far past the data so the
fallback ticks at the data's ends overlapped. The span is now iterated to its
fixed point, and when the end labels would take more than 60% of the plot the
table carries the numbers instead.
@pkg-pr-new

pkg-pr-new Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

comark-annotate

pnpm add https://pkg.pr.new/comark-annotate@97

comark-carousel

pnpm add https://pkg.pr.new/comark-carousel@97

comark-charts

pnpm add https://pkg.pr.new/comark-charts@97

comark-chips

pnpm add https://pkg.pr.new/comark-chips@97

comark-code

pnpm add https://pkg.pr.new/comark-code@97

comark-diff

pnpm add https://pkg.pr.new/comark-diff@97

comark-embed

pnpm add https://pkg.pr.new/comark-embed@97

comark-github

pnpm add https://pkg.pr.new/comark-github@97

comark-icon

pnpm add https://pkg.pr.new/comark-icon@97

comark-include

pnpm add https://pkg.pr.new/comark-include@97

comark-linter

pnpm add https://pkg.pr.new/comark-linter@97

comark-tabs

pnpm add https://pkg.pr.new/comark-tabs@97

comark-zoom

pnpm add https://pkg.pr.new/comark-zoom@97

commit: f764935

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This incremental review covers the changes since the prior Pullfrog review: replacing the capped sequential endpoint padding with a convergent all-row calculation, suppressing labels when the plot is too narrow for them, and printing tied row endpoints only once.

  • Convergent endpoint padding — Derives both scale bounds from all row extrema on each pass, addressing the prior narrow-plot expansion concern.
  • Narrow-plot fallback — Omits endpoint text when its estimated clearance would consume more than 60% of the plot, leaving the data table as the readable fallback.
  • Tied endpoint labels — Uses per-row aggregation and window filtering so repeated endpoint values share one label.
  • Verification — pnpm --filter comark-charts test passes.

Pullfrog  | View workflow run | Using gpt-5.6-sol | 𝕏

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant