Skip to content

feat: Add axis-based tooltips with smooth rendering and crosshair indicator#41

Open
rudi-q wants to merge 13 commits into
mainfrom
feature/axis-tooltip-smooth-rendering
Open

feat: Add axis-based tooltips with smooth rendering and crosshair indicator#41
rudi-q wants to merge 13 commits into
mainfrom
feature/axis-tooltip-smooth-rendering

Conversation

@rudi-q

@rudi-q rudi-q commented Oct 1, 2025

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Axis-based tooltips with optional vertical crosshair and multi-series (multi-point) tooltip display.
    • Configurable trigger modes (point vs. axis) and crosshair styles, color, and width.
    • New “Axis Tooltips” demo screen in the example app with stats and feature highlights.
  • Bug Fixes

    • More robust tick generation for ordinal axes to prevent out-of-range issues.
    • Improved color palette mapping for more consistent series coloring.
  • Tests

    • Comprehensive tests covering axis-triggered tooltips, crosshair rendering, and multi-point tooltip behavior.

@rudi-q rudi-q added the experimental PRs that aren't production ready yet label Oct 1, 2025
@coderabbitai

coderabbitai Bot commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Introduces axis-based tooltip functionality for the charting library, including trigger mode configuration, crosshair rendering, multi-point tooltip support, and interaction detection by X-axis position. Adds new enums, configuration options, widgets, and an example implementation integrated into the chart system.

Changes

Cohort / File(s) Summary
Axis Tooltip Example
example/lib/graphs/axis_tooltip_example.dart
New StatelessWidget example demonstrating axis-triggered tooltips with multi-series quarterly data, crosshair indicator, smooth rendering, and custom color palette with legend styling.
Router and Chart Screen Integration
example/lib/router/app_router.dart, example/lib/screens/chart_screen.dart
Added new navigable route ('/axis-tooltip') with metadata and integrated AxisTooltipExample into chart gallery as index 23, including title, description, and stats-card UI.
Chart Feature List
example/lib/utils/chart_feature_list.dart
Added new case 19 branch returning axis-tooltip feature strings; note: retains existing case 19 for "Legend examples", creating duplicate switch labels that may prevent compilation.
Tooltip Configuration Extension
lib/src/core/chart.dart
Extended TooltipConfigExtension.copyWith with multi-point, trigger mode, and crosshair parameters; added debug logging to customPalette and refactored fallback logic to use modulo-based indexing for unmapped categories.
Interaction Layer—Trigger Modes and Builders
lib/src/interaction/chart_interactions.dart
Added public ChartTooltipTriggerMode (point vs axis), StrokeStyle (solid/dashed/dotted) enums; extended TooltipConfig with new fields (multiPointBuilder, triggerMode, showCrosshair, crosshairColor, crosshairWidth, crosshairStyle); introduced MultiPointTooltipBuilder typedef and DefaultTooltips.multiPoint(...) builder with series-list rendering and color indicators.
Crosshair Rendering
lib/src/interaction/crosshair_widget.dart
New CrosshairOverlay widget rendering vertical crosshair lines with configurable color, width, and style (solid, dashed, dotted) within plot area bounds.
Point Detection by X Position
lib/src/interaction/interaction_detector.dart
Added detectPointsByXPosition(Offset screenPosition) to collect and sort multiple data points by X proximity within epsilon tolerance, enabling multi-point tooltip lookups.
Multi-Point Tooltip Support
lib/src/interaction/tooltip_widget.dart
Extended TooltipController and TooltipMixin with showMultiPointTooltip(...) method; refactored internal state and builder routing to support both single-point and multi-point modes with smart content recreation vs repositioning logic.
Chart Widget—Axis-Mode Interactions
lib/src/widgets/animated_chart_widget.dart
Added crosshair state tracking and conditional rendering; branched hover/pan/gesture handling by trigger mode (axis: detectPointsByXPosition + multi-point tooltips; point: existing single-point logic); updated tooltip visibility checks to cover multi-point builders and added crosshair clearing on exit.
Painter Cleanup
lib/src/widgets/animated_chart_painter.dart
Removed redundant color palette reference expression in _drawGroupedProgressBar.
Comprehensive Test Suite
test/axis_tooltip_test.dart
New tests covering axis-triggered tooltips, crosshair rendering with multiple styles, multi-point tooltip builders, continuous/ordinal scale compatibility, and edge cases (empty points, mouse exit).

Sequence Diagram

sequenceDiagram
    actor User
    participant Chart as AnimatedChartWidget
    participant Detector as InteractionDetector
    participant Tooltip as TooltipWidget
    participant Crosshair as CrosshairOverlay

    User->>Chart: Hover over X-axis (axis mode)
    Chart->>Detector: detectPointsByXPosition(screenPos)
    Detector-->>Chart: List<DataPointInfo> at closest X
    
    alt Points found
        Chart->>Crosshair: Update position (crosshair enabled)
        Crosshair-->>Chart: Vertical line rendered
        Chart->>Tooltip: showMultiPointTooltip(points, pos)
        Tooltip-->>Chart: Multi-point tooltip built & shown
    else No points at X
        Chart->>Crosshair: Clear position
        Chart->>Tooltip: Hide tooltip
    end
    
    User->>Chart: Move pointer along X-axis
    Chart->>Detector: detectPointsByXPosition(newScreenPos)
    Detector-->>Chart: Updated List<DataPointInfo>
    Chart->>Crosshair: Update crosshair X position
    Crosshair-->>Chart: Smooth crosshair movement
    Chart->>Tooltip: Update tooltip position/content
    Tooltip-->>Chart: Reposition or rebuild multi-point display
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • PR #88: Modifies lib/src/widgets/animated_chart_painter.dart alongside this PR's painter changes and color palette logic adjustments.
  • PR #38: Extends TooltipConfigExtension.copyWith API, which this PR also modifies with new tooltip-configuration parameters.
  • PR #58: Modifies lib/src/widgets/animated_chart_widget.dart and animated_chart_painter.dart, overlapping with this PR's chart interaction and rendering updates.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the axis-tooltip feature, its benefits, and any relevant implementation details or testing notes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main feature addition: axis-based tooltips with smooth rendering and a crosshair indicator, which is the core focus of the entire changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/axis-tooltip-smooth-rendering

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@vercel

vercel Bot commented Oct 1, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cristalyse_example Ready Ready Preview, Comment Apr 5, 2026 5:50am

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
lib/src/widgets/animated_chart_widget.dart (1)

329-407: Include multi-point tooltips in the touch guard
hasTooltips only checks tooltip.builder, so when we rely on multiPointBuilder (the core of axis mode) the touch path short-circuits and axis tooltips/crosshair never update during drag on mobile. Please treat multiPointBuilder as a tooltip signal here so axis mode works on touch devices.

Apply this patch:

-    final hasTooltips = widget.interaction.hover?.onHover != null ||
-        widget.interaction.tooltip?.builder != null;
+    final hasTooltips = widget.interaction.hover?.onHover != null ||
+        widget.interaction.tooltip?.builder != null ||
+        widget.interaction.tooltip?.multiPointBuilder != null;
lib/src/interaction/tooltip_widget.dart (1)

216-268: Overlay never updates position/content after first paint
_updateTooltipPosition only calls markNeedsBuild, but the OverlayEntry builder closes over capturedPosition/capturedPoints from the moment the overlay was created. As a result the widget rebuilds with the exact same data and coordinates—tooltips never follow the cursor and never refresh their payload. Either rebuild the overlay with the latest state each time or have the builder read _currentPosition, _currentPoint, and _currentPoints directly from the state so markNeedsBuild can do its job.

One possible fix is:

-    final Offset capturedPosition = _currentPosition!;
-    final TooltipConfig capturedConfig = widget.config;
-
-    // Build tooltip content based on mode
-    Widget tooltipContent;
-    if (_isMultiPoint && capturedConfig.multiPointBuilder != null) {
-      final List<DataPointInfo> capturedPoints = List.from(_currentPoints!);
-      tooltipContent = capturedConfig.multiPointBuilder!(capturedPoints);
-    } else if (!_isMultiPoint && capturedConfig.builder != null) {
-      final DataPointInfo capturedPoint = _currentPoint!;
-      tooltipContent = capturedConfig.builder!(capturedPoint);
-    } else {
-      return; // No appropriate builder
-    }
-
     _overlayEntry = OverlayEntry(
       builder: (context) {
+        final config = widget.config;
+        final position = _currentPosition!;
+        Widget? tooltipContent;
+        if (_isMultiPoint && config.multiPointBuilder != null && _currentPoints != null) {
+          tooltipContent = config.multiPointBuilder!(_currentPoints!);
+        } else if (!_isMultiPoint && config.builder != null && _currentPoint != null) {
+          tooltipContent = config.builder!(_currentPoint!);
+        }
+        if (tooltipContent == null) {
+          return const SizedBox.shrink();
+        }
         return _TooltipPositioned(
-          position: capturedPosition,
-          config: capturedConfig,
+          position: position,
+          config: config,
           fadeAnimation: _fadeAnimation,
           scaleAnimation: _scaleAnimation,
           child: tooltipContent,
         );
       },
     );

That way a simple markNeedsBuild actually reflects the latest pointer location and data set.

🧹 Nitpick comments (1)
lib/src/core/scale.dart (1)

82-82: Consider removing or conditionalizing the debug print.

The debugPrint statement logs every domain update, which could create noise in debug logs if the domain is updated frequently during axis-based tooltip interactions. Additionally, the ellipsis (...) always appears even when the domain has 10 or fewer items.

If this logging is temporary for feature development, consider removing it before merge. If it's intended for ongoing debugging, consider:

  1. Option 1: Remove the debug print entirely:
   set domain(List<dynamic> value) {
     _domain = value;
-    debugPrint('OrdinalScale: Setting domain with ${value.length} items: ${value.take(10)}...');
     _calculateBandWidth();
   }
  1. Option 2: Add a conditional flag or fix the ellipsis formatting:
   set domain(List<dynamic> value) {
     _domain = value;
-    debugPrint('OrdinalScale: Setting domain with ${value.length} items: ${value.take(10)}...');
+    if (kDebugMode) {
+      final preview = value.take(10).toList();
+      final ellipsis = value.length > 10 ? '...' : '';
+      debugPrint('OrdinalScale: Setting domain with ${value.length} items: $preview$ellipsis');
+    }
     _calculateBandWidth();
   }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12cf41a and d5bcc85.

📒 Files selected for processing (12)
  • example/lib/graphs/axis_tooltip_example.dart (1 hunks)
  • example/lib/router/app_router.dart (2 hunks)
  • example/lib/screens/chart_screen.dart (3 hunks)
  • example/lib/utils/chart_feature_list.dart (1 hunks)
  • lib/src/core/chart.dart (4 hunks)
  • lib/src/core/scale.dart (2 hunks)
  • lib/src/interaction/chart_interactions.dart (7 hunks)
  • lib/src/interaction/crosshair_widget.dart (1 hunks)
  • lib/src/interaction/interaction_detector.dart (1 hunks)
  • lib/src/interaction/tooltip_widget.dart (8 hunks)
  • lib/src/widgets/animated_chart_painter.dart (0 hunks)
  • lib/src/widgets/animated_chart_widget.dart (6 hunks)
💤 Files with no reviewable changes (1)
  • lib/src/widgets/animated_chart_painter.dart
🔇 Additional comments (1)
lib/src/core/scale.dart (1)

132-136: LGTM! Defensive bounds check for tick generation.

The clamping prevents potential out-of-bounds access when generating tick indices. While mathematically the computed index (i * step).floor() should always fall within [0, _domain.length - 1] for valid inputs, this defensive check guards against floating-point edge cases and ensures robustness during axis-based tooltip interactions.

The logic correctly samples the domain at regular intervals and the clamp operation does not alter the intended behavior.

Comment thread example/lib/screens/chart_screen.dart
Comment thread lib/src/interaction/interaction_detector.dart Outdated
Comment thread lib/src/interaction/tooltip_widget.dart
Comment thread lib/src/widgets/animated_chart_widget.dart
rudi-q added 5 commits October 1, 2025 16:27
- Remove xTolerance parameter to ensure tooltips always appear
- Implement two-pass algorithm to find closest X position
- Group points at same X using 2px epsilon for web precision
- Sort points by Y position (top to bottom) for consistent ordering

This ensures axis-based tooltips display continuously as users hover
across the chart, rather than only appearing within a tolerance range.
- Clear crosshair position when pan gesture ends
- Always hide tooltip on gesture end (both single and multi-point)
- Remove xTolerance parameter from detectPointsByXPosition calls

Fixes issue where crosshair and tooltip would linger after interaction
ended, improving visual clarity and user experience.
Replace deprecated withOpacity() with withValues(alpha: 0.9) to avoid
deprecation warnings in Flutter 3.27+.
Add proper metadata for the axis tooltip example in chart_screen.dart
so it displays correctly in the example app navigation.
Add 14 widget tests covering:
- Chart rendering with axis tooltip configuration
- Point detection at X positions during hover
- Crosshair visibility and behavior
- Custom multi-point tooltip builders
- Axis mode with continuous and ordinal X scales
- Different crosshair styles (solid, dashed, dotted)
- Empty points handling
- Axis vs point mode comparison
- Nearest X position snapping behavior

All tests pass with 100% coverage of axis tooltip functionality.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
lib/src/widgets/animated_chart_widget.dart (1)

327-336: Restore axis tooltips for touch interactions.

Line 329’s hasTooltips flag only looks at builder, so configs that rely solely on multiPointBuilder never enter the pan tooltip branch. That means axis-mode tooltips (and the crosshair) stop updating as soon as the user drags on touch devices—the new feature breaks in its most common setup. Please treat multiPointBuilder as a tooltip signal (and reuse the computed config inside the branch) so axis tooltips keep working during pan gestures.

-    final panConfig = widget.interaction.pan;
-    final hasTooltips = widget.interaction.hover?.onHover != null ||
-        widget.interaction.tooltip?.builder != null;
+    final panConfig = widget.interaction.pan;
+    final tooltipConfig = widget.interaction.tooltip;
+    final hasTooltips = widget.interaction.hover?.onHover != null ||
+        tooltipConfig?.builder != null ||
+        tooltipConfig?.multiPointBuilder != null;
@@
-    if (hasTooltips && panConfig?.enabled != true) {
+    if (hasTooltips && panConfig?.enabled != true) {
@@
-      final tooltipConfig = widget.interaction.tooltip;
       final useAxisMode =
           tooltipConfig?.triggerMode == ChartTooltipTriggerMode.axis;
lib/src/interaction/tooltip_widget.dart (1)

248-271: Rebuild uses stale coordinates—tooltip never repositions
_updateTooltipPosition updates _currentPosition and calls markNeedsBuild, but _createTooltip captured the position in a local final so _TooltipPositioned keeps the old value forever. The tooltip appears frozen while the user moves along the axis. Read _currentPosition inside the builder (or derive an up-to-date position there) instead of capturing it once.

Here’s one way to fix it:

-    final Offset capturedPosition = _currentPosition!;
-    final TooltipConfig capturedConfig = widget.config;
+    final TooltipConfig capturedConfig = widget.config;
@@
-        return _TooltipPositioned(
-          position: capturedPosition,
+        final position = _currentPosition!;
+
+        return _TooltipPositioned(
+          position: position,
♻️ Duplicate comments (2)
lib/src/interaction/interaction_detector.dart (1)

92-116: Still mixing multiple X buckets on ties

When the pointer sits midway between two X buckets (e.g., two bands centred 15 px away), both yield the same minXDistance, so the second pass keeps both sets. The tooltip/crosshair resumes aggregating unrelated X values—the same problem flagged earlier. Instead, capture the screen X of the closest point and only include points that land on that pixel (within epsilon). Please adjust as follows:

-    double minXDistance = double.infinity;
-    for (final point in _allPoints) {
-      final xDistance = (screenPosition.dx - point.screenPosition.dx).abs();
-      if (xDistance < minXDistance) {
-        minXDistance = xDistance;
-      }
-    }
+    double minXDistance = double.infinity;
+    double? targetX;
+    for (final point in _allPoints) {
+      final xDistance = (screenPosition.dx - point.screenPosition.dx).abs();
+      if (xDistance < minXDistance) {
+        minXDistance = xDistance;
+        targetX = point.screenPosition.dx;
+      }
+    }
@@
-    for (final point in _allPoints) {
-      final xDistance = (screenPosition.dx - point.screenPosition.dx).abs();
-      if ((xDistance - minXDistance).abs() <= epsilon) {
-        pointsAtX.add(point);
-      }
-    }
+    if (targetX == null) return [];
+
+    for (final point in _allPoints) {
+      if ((point.screenPosition.dx - targetX).abs() <= epsilon) {
+        pointsAtX.add(point);
+      }
+    }
lib/src/interaction/tooltip_widget.dart (1)

115-149: Maintain previous tooltip state before diffing
We’re still mutating _currentPoint/_currentPoints before comparing, so _hasSameData* always sees identical objects and never recreates the overlay. Axis hover sticks on the first X value and won’t refresh—same regression we discussed earlier. Capture the previous state first, then update the fields only after deciding whether the payload actually changed.

Apply this diff to restore correct behavior:

   void _showTooltipInternal({
     DataPointInfo? singlePoint,
     List<DataPointInfo>? multiPoints,
     required Offset position,
     required bool isMultiPoint,
   }) {
-    // Update current state
-    _currentPoint = singlePoint;
-    _currentPoints = multiPoints;
+    final previousPoint = _currentPoint;
+    final previousPoints = _currentPoints;
+
+    // Update current state
+    _currentPoint = singlePoint;
+    _currentPoints = multiPoints;
@@
-      final shouldRecreate = isMultiPoint
-          ? !_hasSameDataMulti(_currentPoints, multiPoints)
-          : !_hasSameData(_currentPoint, singlePoint);
+      final shouldRecreate = isMultiPoint
+          ? !_hasSameDataMulti(previousPoints, multiPoints)
+          : !_hasSameData(previousPoint, singlePoint);
🧹 Nitpick comments (1)
lib/src/core/scale.dart (1)

82-83: Refine the debug log message for clarity.

The debug log provides useful diagnostic information for domain mutations. However, the "..." suffix is misleading when value.length <= 10. Consider conditionally appending it or removing it entirely.

Apply this diff for a more accurate log message:

-    debugPrint(
-        'OrdinalScale: Setting domain with ${value.length} items: ${value.take(10)}...');
+    debugPrint(
+        'OrdinalScale: Setting domain with ${value.length} items: ${value.take(10)}${value.length > 10 ? '...' : ''}');
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5bcc85 and 9547e2b.

📒 Files selected for processing (10)
  • example/lib/graphs/axis_tooltip_example.dart (1 hunks)
  • example/lib/screens/chart_screen.dart (5 hunks)
  • lib/src/core/chart.dart (4 hunks)
  • lib/src/core/scale.dart (2 hunks)
  • lib/src/interaction/chart_interactions.dart (7 hunks)
  • lib/src/interaction/crosshair_widget.dart (1 hunks)
  • lib/src/interaction/interaction_detector.dart (1 hunks)
  • lib/src/interaction/tooltip_widget.dart (8 hunks)
  • lib/src/widgets/animated_chart_widget.dart (6 hunks)
  • test/axis_tooltip_test.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • example/lib/screens/chart_screen.dart
🔇 Additional comments (1)
lib/src/core/scale.dart (1)

133-137: LGTM!

The clamping logic is a solid defensive measure that prevents potential index-out-of-bounds errors. While mathematically the floor operation should keep indices within bounds, the clamp guards against floating-point precision edge cases and makes the code more robust.

Comment thread lib/src/interaction/chart_interactions.dart
Comment thread lib/src/interaction/chart_interactions.dart
rudi-q added 2 commits October 1, 2025 16:41
- Add default multiPointBuilder to axisConfig so axis tooltips work OOTB
- Fix yValue fallback to use point.yValue or point.data['y'] when yColumn is null
- Remove 'const' from axisConfig to allow non-const multiPointBuilder

This ensures axis tooltips display meaningful values even when columns
aren't explicitly specified, and work immediately with TooltipConfig.axisConfig.
@rudi-q

rudi-q commented Oct 1, 2025

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 1, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9547e2b and c51957e.

📒 Files selected for processing (2)
  • lib/src/interaction/chart_interactions.dart (7 hunks)
  • lib/src/interaction/interaction_detector.dart (1 hunks)
🔇 Additional comments (10)
lib/src/interaction/interaction_detector.dart (6)

75-82: LGTM! Clear documentation for axis-based tooltip detection.

The method signature and documentation clearly explain the intended behavior: snapping to the nearest X position without tolerance limits and returning all points at that X sorted by Y value.


83-90: LGTM! Proper initialization and early exit.

The index building and empty-check logic follows the same reliable pattern as detectPoint.


92-99: LGTM! Correct minimum distance calculation.

The loop correctly identifies the minimum X distance across all points.


101-115: LGTM! Epsilon-based grouping handles multi-series and rendering precision.

The epsilon tolerance (2.0px) correctly groups all points that share approximately the same X position, accommodating web rendering precision while preventing cross-bucket mixing.


117-118: LGTM! Proper empty-result handling.


120-135: LGTM! Consistent sorting and conversion logic.

The Y-based sort ensures predictable tooltip ordering, and the conversion to DataPointInfo matches the pattern used in detectPoint.

lib/src/interaction/chart_interactions.dart (4)

3-18: LGTM! Well-defined enums with clear documentation.

The trigger mode and stroke style enums are appropriately scoped and well-documented.


43-86: LGTM! TooltipConfig extended with axis-mode fields.

The new fields (multiPointBuilder, triggerMode, crosshair options) are well-typed, documented, and have sensible defaults that preserve backward compatibility.


96-106: LGTM! axisConfig properly wired with multiPointBuilder.

The default axis configuration correctly enables crosshair and provides the multi-point builder, addressing the previous review concern.


175-175: LGTM! Correct typedef for multi-point tooltips.

Comment thread lib/src/interaction/chart_interactions.dart
@rudi-q rudi-q added the enhancement New feature or request label Oct 1, 2025
CRITICAL BUG FIX:
- Capture previous state BEFORE overwriting in _showTooltipInternal
- Compare new data against previous data (not against itself!)
- Update state AFTER comparison decision is made

This fixes the major issue where axis tooltips would get stuck on the
first X position because every subsequent hover was treated as 'same data'
due to comparing new state against itself.

Also:
- Change empty string fallback to 'N/A' for missing values (better UX)

This was breaking the entire axis tooltip feature - tooltips would appear
once and never update as the user moved their mouse horizontally.
@rudi-q

rudi-q commented Oct 1, 2025

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 1, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@rudi-q rudi-q added contributions welcome Extra attention is needed and removed contributions welcome Extra attention is needed labels Oct 1, 2025
@jbbjarnason

Copy link
Copy Markdown
Contributor

This looks good, will it be possible to make a crosshair indicator(the dashed line) be stickable and have more than one? Than a user can drag or place a indicator and compare two or more points on the chart more easily.

@rudi-q

rudi-q commented Oct 12, 2025

Copy link
Copy Markdown
Owner Author

This looks good, will it be possible to make a crosshair indicator(the dashed line) be stickable and have more than one? Than a user can drag or place a indicator and compare two or more points on the chart more easily.

That's a good idea. It would be useful to have implement that. Just need to resolve the conflicts here first.

@jbbjarnason

Copy link
Copy Markdown
Contributor

It would also be great if export to svg would support including the dragged indicator/s

@jbbjarnason

Copy link
Copy Markdown
Contributor

I am really looking forward to this feature, do you know when you will continue with it?

…art index 23

- Reassign AxisTooltipExample from chart index 19 to 23 (legends/time/zoom/combo added at 19-22 in main)
- Add /axis-tooltip GoRoute after /combo-bar-line in router
- Keep PR's modulo-safe color palette mapping (prevents index out-of-bounds)
- Take main's improved OrdinalScale.getTicks() (pixel-aware label density)
- Take main's defensive List.from(value) in OrdinalScale.domain setter
- Keep PR's _showTooltipInternal refactor and showMultiPointTooltip (required for axis tooltip feature)
- Keep PR's axis-mode hit detection structure in animated_chart_widget (hitRadius lives in else branch)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/src/widgets/animated_chart_widget.dart (1)

417-420: ⚠️ Potential issue | 🟠 Major

Include multiPointBuilder in the touch-tooltip gate.

TooltipConfig.axisConfig relies on multiPointBuilder without a single-point builder. With the current hasTooltips check, the axis tooltip path is skipped on touch/drag unless a hover callback is also set.

Suggested fix
-    final hasTooltips = widget.interaction.hover?.onHover != null ||
-        widget.interaction.tooltip?.builder != null;
+    final hasTooltips = widget.interaction.hover?.onHover != null ||
+        widget.interaction.tooltip?.builder != null ||
+        widget.interaction.tooltip?.multiPointBuilder != null;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/src/widgets/animated_chart_widget.dart` around lines 417 - 420, The
tooltip presence check in the widget (the hasTooltips boolean built from
panConfig and widget.interaction.hover?.onHover /
widget.interaction.tooltip?.builder) omits cases where TooltipConfig.axisConfig
uses a multiPointBuilder without a single-point builder; update the gate to also
consider widget.interaction.tooltip?.multiPointBuilder != null so touch/drag
axis tooltips run when multiPointBuilder is provided (change the hasTooltips
calculation that references widget.interaction.hover and
widget.interaction.tooltip to include multiPointBuilder).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@example/lib/utils/chart_feature_list.dart`:
- Around line 165-172: The switch branch currently uses case 19 for the
axis-tooltip feature list but the screen is wired to chartIndex: 23, so the
axis-tooltip branch is never reached; update the switch to use case 23 instead
of case 19 (locate the switch that returns the list for the axis-based tooltip —
the branch labeled "case 19: // Axis tooltips") so the axis-tooltip feature list
is returned when chartIndex is 23.

In `@lib/src/core/chart.dart`:
- Around line 1191-1194: In customPalette(), remove the debugPrint that emits
the raw categories array (the call that prints 'CustomPalette: Found
${categories.length} categories: $categories') so caller-provided series labels
are not logged; if you need diagnostic output keep only non-sensitive info
(e.g., log the count via '${categories.length}' or the existing
this.colorPalette.length line) and ensure no variable interpolation of
categories or other caller data remains in debugPrint calls.

In `@lib/src/interaction/tooltip_widget.dart`:
- Around line 231-237: The overlay builder is closing over the initial
capturedPosition so calling _updateTooltipPosition and markNeedsBuild() doesn't
move the tooltip; change the overlay builder to read the mutable state position
(use _currentPosition) instead of the local capturedPosition so rebuilds reflect
the updated coordinates, and update the other overlay creation site (the other
builder referenced in the comment range) to the same pattern; keep using
_overlayEntry!.markNeedsBuild() in _updateTooltipPosition to trigger the
rebuild.

In `@lib/src/widgets/animated_chart_widget.dart`:
- Around line 467-505: The _handlePanUpdate() implementation is using a stale
details variable (e.g. details.localPosition) although the method only receives
a localPosition parameter; replace all uses of details.localPosition with the
provided localPosition (use in calls to
_interactionDetector.detectPointsByXPosition, _interactionDetector.detectPoint,
in setState for _crosshairPosition, and when feeding tooltip logic like
_showMultiPointTooltip/hideTooltip) and keep globalPosition usage as-is if it
comes from panContext; update widget.interaction.hover?.onHover to pass the
detected point derived from localPosition. Ensure all references to details are
removed so _handlePanUpdate(), useAxisMode, _interactionDetector,
_crosshairPosition, _showMultiPointTooltip, hideTooltip and
widget.interaction.hover compile cleanly.

---

Outside diff comments:
In `@lib/src/widgets/animated_chart_widget.dart`:
- Around line 417-420: The tooltip presence check in the widget (the hasTooltips
boolean built from panConfig and widget.interaction.hover?.onHover /
widget.interaction.tooltip?.builder) omits cases where TooltipConfig.axisConfig
uses a multiPointBuilder without a single-point builder; update the gate to also
consider widget.interaction.tooltip?.multiPointBuilder != null so touch/drag
axis tooltips run when multiPointBuilder is provided (change the hasTooltips
calculation that references widget.interaction.hover and
widget.interaction.tooltip to include multiPointBuilder).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: db872562-dcd6-4666-a1fc-5e7d7edeb953

📥 Commits

Reviewing files that changed from the base of the PR and between 9d78204 and 4bea07a.

📒 Files selected for processing (8)
  • example/lib/router/app_router.dart
  • example/lib/screens/chart_screen.dart
  • example/lib/utils/chart_feature_list.dart
  • lib/src/core/chart.dart
  • lib/src/interaction/chart_interactions.dart
  • lib/src/interaction/tooltip_widget.dart
  • lib/src/widgets/animated_chart_painter.dart
  • lib/src/widgets/animated_chart_widget.dart
💤 Files with no reviewable changes (1)
  • lib/src/widgets/animated_chart_painter.dart

Comment thread example/lib/utils/chart_feature_list.dart Outdated
Comment thread lib/src/core/chart.dart
Comment on lines +231 to +237
/// Update tooltip position without recreating
void _updateTooltipPosition(Offset newPosition) {
if (_overlayEntry != null && mounted) {
_currentPosition = newPosition;
// Force overlay to rebuild with new position
_overlayEntry!.markNeedsBuild();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same-data tooltip moves still render at the original position.

_updateTooltipPosition() only marks the entry dirty, but the overlay builder closes over capturedPosition. As a result, the tooltip stays at its first location until the payload changes, so the “smooth” path never actually follows the pointer.

Suggested fix
         return _TooltipPositioned(
-          position: capturedPosition,
+          position: _currentPosition ?? capturedPosition,
           config: capturedConfig,
           fadeAnimation: _fadeAnimation,
           scaleAnimation: _scaleAnimation,
           child: tooltipContent,
         );

Also applies to: 260-283

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/src/interaction/tooltip_widget.dart` around lines 231 - 237, The overlay
builder is closing over the initial capturedPosition so calling
_updateTooltipPosition and markNeedsBuild() doesn't move the tooltip; change the
overlay builder to read the mutable state position (use _currentPosition)
instead of the local capturedPosition so rebuilds reflect the updated
coordinates, and update the other overlay creation site (the other builder
referenced in the comment range) to the same pattern; keep using
_overlayEntry!.markNeedsBuild() in _updateTooltipPosition to trigger the
rebuild.

Comment thread lib/src/widgets/animated_chart_widget.dart Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributions welcome Extra attention is needed enhancement New feature or request experimental PRs that aren't production ready yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants