feat(component): add rule-based styling for Graph and Map charts - #278
Merged
alfredo1996 merged 1 commit intoMar 30, 2026
Merged
Conversation
Closes #259 Graph chart: - Add stylingRules/paramValues props to GraphChartProps - Evaluate rules against node.value in toNvlNode() - Rule color takes priority over explicit node.color and label palette - 3 new tests: rule match, priority over explicit color, no-value skip Map chart: - Add stylingRules/paramValues props to MapChartProps - Evaluate rules against marker.value in marker creation loop - Rule color takes priority over explicit marker.color - 3 new tests (TDD: written RED first, then implemented GREEN) Registry + renderer: - Set supportsStyling: true for graph and map - Add stylingTargets with "Node Color" / "Marker Color" - Forward stylingRules/paramValues in chart-renderer.tsx - Update 4 existing tests that expected graph/map to be unstyled Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughAdded rule-based styling support to Graph and Map charts by introducing Changes
Sequence DiagramsequenceDiagram
participant ChartRenderer
participant GraphChart/MapChart
participant StylingRuleEngine
participant RenderOutput
ChartRenderer->>GraphChart/MapChart: pass stylingRules & paramValues
GraphChart/MapChart->>GraphChart/MapChart: iterate nodes/markers
GraphChart/MapChart->>StylingRuleEngine: resolveStylingRuleColor(value, rules, params)
StylingRuleEngine->>StylingRuleEngine: evaluate value against rule operators
StylingRuleEngine-->>GraphChart/MapChart: return rule color (if match)
GraphChart/MapChart->>GraphChart/MapChart: apply color precedence:<br/>rule color → explicit color → default
GraphChart/MapChart->>RenderOutput: render styled nodes/markers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
alfredo1996
changed the base branch from
release/1.0
to
improve/charts-integration
March 30, 2026 21:16
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
component/src/charts/graph-chart.tsx (1)
130-133: Update stale docs to match new precedence and scope.After this change, color priority is rule → explicit
node.color→ label palette, and styling here affects color (not size). Please align the comments around Line 130 and Line 227 to avoid drift.Also applies to: 227-229, 256-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@component/src/charts/graph-chart.tsx` around lines 130 - 133, Update the stale JSDoc/comments to reflect the new styling precedence and scope: change the comment for stylingRules to state that these rule-based styling entries now affect node color only (not size) and that color precedence is rule → explicit node.color → label palette; update the paramValues comment to note these are resolved parameter values used during rule evaluation; then apply the same wording changes to the other comment blocks around the styling-related docs (the blocks currently at the second location and the third location) so all descriptions consistently state color-only effect and the new precedence order.app/src/lib/chart-registry.ts (1)
744-753: Registry update looks correct; consider syncing interface docs.
graph/mapnow expose styling targets, but thesupportsStylingdoc comment at Line 61-64 still lists them as unsupported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/lib/chart-registry.ts` around lines 744 - 753, The supportsStyling doc comment is out of sync with the registry: chart types graph and map now expose stylingTargets, so update the supportsStyling comment to list graph and map as supporting styling; specifically edit the supportsStyling docblock (the comment above the supportsStyling definition) to mention "graph" and "map" alongside any other supported types, and ensure the description references stylingTargets on the ChartRegistry entries (e.g., the graph and map objects with stylingTargets) so the docs reflect the current registry behavior.component/src/charts/map-chart.tsx (1)
43-46: Tighten prop docs: rules currently affect marker color, not size.The comment says “marker color/size,” but this change only resolves color (Lines 205-214). Please update wording to avoid API confusion.
Proposed doc-only fix
- /** Rule-based styling rules for marker color/size */ + /** Rule-based styling rules for marker color */ stylingRules?: StylingRule[];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@component/src/charts/map-chart.tsx` around lines 43 - 46, The prop documentation is misleading: update the comment on stylingRules in map-chart.tsx to remove "size" and state that rules control marker color only (e.g., change "Rule-based styling rules for marker color/size" to "Rule-based styling rules for marker color"); also ensure the paramValues comment still clarifies they are "Resolved parameter values for styling rule evaluation" and reference the stylingRules symbol so readers know these values are used when evaluating stylingRules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/lib/chart-registry.ts`:
- Around line 744-753: The supportsStyling doc comment is out of sync with the
registry: chart types graph and map now expose stylingTargets, so update the
supportsStyling comment to list graph and map as supporting styling;
specifically edit the supportsStyling docblock (the comment above the
supportsStyling definition) to mention "graph" and "map" alongside any other
supported types, and ensure the description references stylingTargets on the
ChartRegistry entries (e.g., the graph and map objects with stylingTargets) so
the docs reflect the current registry behavior.
In `@component/src/charts/graph-chart.tsx`:
- Around line 130-133: Update the stale JSDoc/comments to reflect the new
styling precedence and scope: change the comment for stylingRules to state that
these rule-based styling entries now affect node color only (not size) and that
color precedence is rule → explicit node.color → label palette; update the
paramValues comment to note these are resolved parameter values used during rule
evaluation; then apply the same wording changes to the other comment blocks
around the styling-related docs (the blocks currently at the second location and
the third location) so all descriptions consistently state color-only effect and
the new precedence order.
In `@component/src/charts/map-chart.tsx`:
- Around line 43-46: The prop documentation is misleading: update the comment on
stylingRules in map-chart.tsx to remove "size" and state that rules control
marker color only (e.g., change "Rule-based styling rules for marker color/size"
to "Rule-based styling rules for marker color"); also ensure the paramValues
comment still clarifies they are "Resolved parameter values for styling rule
evaluation" and reference the stylingRules symbol so readers know these values
are used when evaluating stylingRules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a3d9cab-342a-470e-8343-77448e19690a
📒 Files selected for processing (9)
app/src/components/chart-renderer.tsxapp/src/lib/__tests__/chart-registry.test.tsapp/src/lib/__tests__/widget-actions.test.tsapp/src/lib/chart-registry.tsapp/src/stores/__tests__/widget-editor-store.test.tscomponent/src/charts/__tests__/graph-chart.test.tsxcomponent/src/charts/__tests__/map-chart.test.tsxcomponent/src/charts/graph-chart.tsxcomponent/src/charts/map-chart.tsx
|
This was referenced Mar 31, 2026
3 tasks
alfredo1996
added a commit
that referenced
this pull request
May 10, 2026
Closes #259 Graph chart: - Add stylingRules/paramValues props to GraphChartProps - Evaluate rules against node.value in toNvlNode() - Rule color takes priority over explicit node.color and label palette - 3 new tests: rule match, priority over explicit color, no-value skip Map chart: - Add stylingRules/paramValues props to MapChartProps - Evaluate rules against marker.value in marker creation loop - Rule color takes priority over explicit marker.color - 3 new tests (TDD: written RED first, then implemented GREEN) Registry + renderer: - Set supportsStyling: true for graph and map - Add stylingTargets with "Node Color" / "Marker Color" - Forward stylingRules/paramValues in chart-renderer.tsx - Update 4 existing tests that expected graph/map to be unstyled Co-authored-by: alfredorubin96 <alfredo.rubin@neotechnology.com> 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
stylingRulesprop, evaluated againstnode.valuestylingRulesprop, evaluated againstmarker.valuesupportsStyling: truefor both, withstylingTargetsstylingRules/paramValuesto Graph and Map casesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests