feat(component): add reference lines (markLine) for bar and line charts - #173
Closed
alfredo1996 wants to merge 4 commits into
Closed
feat(component): add reference lines (markLine) for bar and line charts#173alfredo1996 wants to merge 4 commits into
alfredo1996 wants to merge 4 commits into
Conversation
Add parseReferenceLines() and buildMarkLineFromRefs() to chart-utils.ts.
Wire markLine into BarChart — reference lines attach to first series.
Register MarkLineComponent in ECharts and update all test mocks.
- JSON config: [{"value":50,"label":"Target","color":"#ff0000"}]
- Chart option added to bar and line in chart-options-schema
- 7 new tests for reference line parsing
Closes #136
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alfredo1996
added
enhancement
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughAdded support for horizontal reference lines in bar and line charts via ECharts markLine. Introduced Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
component/src/charts/chart-utils.ts (2)
11-15: Consider extendingReferenceLinefor vertical line support.Per issue
#136acceptance criteria ("Support both horizontal and vertical lines"), consider adding an optionalaxis?: 'x' | 'y'field to enable vertical reference lines viaxAxispositioning.export interface ReferenceLine { value: number; label?: string; color?: string; + /** 'y' for horizontal lines (default), 'x' for vertical lines */ + axis?: 'x' | 'y'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@component/src/charts/chart-utils.ts` around lines 11 - 15, Extend the ReferenceLine interface to support vertical lines by adding an optional axis?: 'x' | 'y' property so consumers can specify axis orientation; update any consumers of ReferenceLine (e.g., chart rendering logic that uses value and xAxis/yAxis) to treat axis === 'x' as a vertical line anchored to the xAxis and axis === 'y' (or undefined) as the existing horizontal behavior, and adjust type annotations and tests accordingly to accept the new field.
41-58:buildMarkLineFromRefsonly supports horizontal lines.Currently hardcodes
yAxispositioning. To support vertical lines per issue#136:♻️ Proposed refactor for axis flexibility
export function buildMarkLineFromRefs(lines: ReferenceLine[]) { if (!lines.length) return undefined; return { silent: true, symbol: "none", data: lines.map((line) => ({ - yAxis: line.value, + ...(line.axis === 'x' ? { xAxis: line.value } : { yAxis: line.value }), label: { formatter: line.label ?? String(line.value), position: "insideEndTop" as const, }, lineStyle: { color: line.color ?? "#888", type: "dashed" as const, }, })), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@component/src/charts/chart-utils.ts` around lines 41 - 58, buildMarkLineFromRefs currently hardcodes horizontal lines by always emitting yAxis; update buildMarkLineFromRefs to support vertical lines by reading an axis/orientation property on each ReferenceLine (e.g., axis: 'x' | 'y' or orientation: 'vertical' | 'horizontal') and conditionally emitting xAxis: line.value for verticals and yAxis: line.value for horizontals (default to 'y' for backward compatibility). Adjust the per-line label.position (choose a sensible default per axis) and keep existing lineStyle/label.formatter logic; ensure the function still returns undefined for empty arrays and preserves the overall returned object shape.component/src/charts/__tests__/reference-line.test.ts (1)
1-47: Missing test coverage forbuildMarkLineFromRefs.
parseReferenceLineshas solid coverage, butbuildMarkLineFromRefscontains non-trivial logic (empty array →undefined, default color#888, label fallback, dashed style) with zero tests.Would you like me to generate test cases for
buildMarkLineFromRefs?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@component/src/charts/__tests__/reference-line.test.ts` around lines 1 - 47, The test suite is missing coverage for buildMarkLineFromRefs which contains important branching (returns undefined for empty input, applies default color "#888", uses label fallback, and sets dashed style) — add unit tests that call buildMarkLineFromRefs with (1) undefined/empty array and assert it returns undefined, (2) a single ReferenceLine without color and without label to verify default color "#888" and correct label fallback, (3) multiple reference lines to verify the produced markLine object contains expected data entries, and (4) an entry with dashed=true to assert the style is dashed; reference the buildMarkLineFromRefs function in the tests to locate the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@component/src/charts/bar-chart.tsx`:
- Around line 39-40: The current referenceLines prop only produces horizontal
marks via yAxis in buildMarkLineFromRefs; add an axis (or direction) field to
the ReferenceLine shape (accepting 'x'|'y' or 'vertical'|'horizontal') and
update buildMarkLineFromRefs to branch on that field: when axis === 'y' keep the
existing yAxis positioning for horizontal lines, and when axis === 'x' generate
a mark line positioned on xAxis (vertical line) using the provided value and
label/color handling; also validate/backward-compatibly default missing axis to
'y' so existing JSON strings still render as horizontal lines. Ensure you
reference the referenceLines prop parsing and the buildMarkLineFromRefs function
to locate and modify the logic.
In `@component/src/components/composed/chart-options-schema.ts`:
- Line 49: The schema exposes referenceLines but LineChart lacks support; update
the LineChart component to accept and process a referenceLines prop (same shape
as BarChart's referenceLines) by adding the prop to its props/type (e.g., in the
LineChart component signature and lineOptions handling), parse/validate the JSON
if required, and render horizontal reference lines (mirror the implementation
pattern used in BarChart) so configured lines appear on the chart; ensure
defaulting when referenceLines is empty and reuse any existing helper/util used
by BarChart for consistency.
---
Nitpick comments:
In `@component/src/charts/__tests__/reference-line.test.ts`:
- Around line 1-47: The test suite is missing coverage for buildMarkLineFromRefs
which contains important branching (returns undefined for empty input, applies
default color "#888", uses label fallback, and sets dashed style) — add unit
tests that call buildMarkLineFromRefs with (1) undefined/empty array and assert
it returns undefined, (2) a single ReferenceLine without color and without label
to verify default color "#888" and correct label fallback, (3) multiple
reference lines to verify the produced markLine object contains expected data
entries, and (4) an entry with dashed=true to assert the style is dashed;
reference the buildMarkLineFromRefs function in the tests to locate the
implementation.
In `@component/src/charts/chart-utils.ts`:
- Around line 11-15: Extend the ReferenceLine interface to support vertical
lines by adding an optional axis?: 'x' | 'y' property so consumers can specify
axis orientation; update any consumers of ReferenceLine (e.g., chart rendering
logic that uses value and xAxis/yAxis) to treat axis === 'x' as a vertical line
anchored to the xAxis and axis === 'y' (or undefined) as the existing horizontal
behavior, and adjust type annotations and tests accordingly to accept the new
field.
- Around line 41-58: buildMarkLineFromRefs currently hardcodes horizontal lines
by always emitting yAxis; update buildMarkLineFromRefs to support vertical lines
by reading an axis/orientation property on each ReferenceLine (e.g., axis: 'x' |
'y' or orientation: 'vertical' | 'horizontal') and conditionally emitting xAxis:
line.value for verticals and yAxis: line.value for horizontals (default to 'y'
for backward compatibility). Adjust the per-line label.position (choose a
sensible default per axis) and keep existing lineStyle/label.formatter logic;
ensure the function still returns undefined for empty arrays and preserves the
overall returned object shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e161b6a4-226c-4a66-ba38-bd09bf97785a
📒 Files selected for processing (7)
component/src/charts/__tests__/base-chart.test.tsxcomponent/src/charts/__tests__/reference-line.test.tscomponent/src/charts/bar-chart.tsxcomponent/src/charts/base-chart.tsxcomponent/src/charts/chart-utils.tscomponent/src/components/composed/chart-options-schema.tscomponent/vitest.setup.ts
Comment on lines
+39
to
+40
| /** JSON string of reference lines: [{ value, label?, color? }] */ | ||
| referenceLines?: string; |
There was a problem hiding this comment.
Issue #136 requires vertical lines support.
The acceptance criteria states "Support both horizontal and vertical lines." Current implementation only supports horizontal lines (yAxis positioning in buildMarkLineFromRefs).
Consider adding an axis or direction field to ReferenceLine to support xAxis positioning for vertical lines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@component/src/charts/bar-chart.tsx` around lines 39 - 40, The current
referenceLines prop only produces horizontal marks via yAxis in
buildMarkLineFromRefs; add an axis (or direction) field to the ReferenceLine
shape (accepting 'x'|'y' or 'vertical'|'horizontal') and update
buildMarkLineFromRefs to branch on that field: when axis === 'y' keep the
existing yAxis positioning for horizontal lines, and when axis === 'x' generate
a mark line positioned on xAxis (vertical line) using the provided value and
label/color handling; also validate/backward-compatibly default missing axis to
'y' so existing JSON strings still render as horizontal lines. Ensure you
reference the referenceLines prop parsing and the buildMarkLineFromRefs function
to locate and modify the logic.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wire up referenceLines prop in LineChart following the same pattern as BarChart (buildMarkLineFromRefs → markLine option) - Add unit tests for buildMarkLineFromRefs covering empty input, defaults, custom colors, and label text - Increases new code coverage toward 80% target Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The typeInEditor fixture was retrying until timeout when CM6's internal view.state.readOnly was true. Now falls through to the keyboard fallback strategy instead of throwing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
alfredo1996
pushed a commit
that referenced
this pull request
Mar 24, 2026
…els, markLine, pie donut) Merges PRs #169, #170, #171, #173, #174 into a single release branch. Resolves merge conflicts in chart-utils.ts, bar-chart.tsx, line-chart.tsx, and chart-options-schema.ts. Includes: - Number formatting for single-value and tooltips (#169) - DataZoom support for bar and line charts (#170) - Auto-rotate and truncate axis labels (#171) - Reference lines (markLine) for bar and line charts (#173) - Donut center text and Top-N grouping for pie chart (#174) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 tasks
Owner
Author
|
Superseded by consolidated PR #185 (release/chart-improvements) |
alfredo1996
pushed a commit
that referenced
this pull request
May 10, 2026
…els, markLine, pie donut) Merges PRs #169, #170, #171, #173, #174 into a single release branch. Resolves merge conflicts in chart-utils.ts, bar-chart.tsx, line-chart.tsx, and chart-options-schema.ts. Includes: - Number formatting for single-value and tooltips (#169) - DataZoom support for bar and line charts (#170) - Auto-rotate and truncate axis labels (#171) - Reference lines (markLine) for bar and line charts (#173) - Donut center text and Top-N grouping for pie chart (#174) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Support horizontal reference lines via ECharts markLine for goals, thresholds, and averages on bar and line charts.
Changes
parseReferenceLines(),buildMarkLineFromRefs()in chart-utils.tsreferenceLinesprop, markLine on first seriesMarkLineComponentMarkLineComponentCloses #136
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests