Skip to content

fix: Constrain LineIndicator within path area boundaries#13

Merged
hectoruch merged 1 commit intocmsfrom
bugfix/line-indicator-limit
Dec 11, 2025
Merged

fix: Constrain LineIndicator within path area boundaries#13
hectoruch merged 1 commit intocmsfrom
bugfix/line-indicator-limit

Conversation

@miguelgarglez
Copy link
Copy Markdown
Collaborator

@miguelgarglez miguelgarglez commented Dec 10, 2025

📝 Summary

This PR fixes an issue where the LineIndicator component could exceed the chart's path boundaries, extending into axis labels and other chart areas. Previously, the indicator was rendered without boundary validation, causing visual overflow and accessibility issues.

The fix adds proper boundary constraints that ensure the line indicator only renders within the valid data visualization area, preventing overflow into axis labels while maintaining consistent behavior across all chart interactions.

🔧 Changes

✅ Added boundary validation to prevent LineIndicator from exceeding xAxis coordinates
✅ Implemented isCursorWithinPathArea check to constrain indicator rendering within data area
✅ Updated y-axis coordinate calculation to use pre-calculated axis values instead of computing from canvas height
✅ Refactored coordinate logic for improved maintainability and consistency
✅ Added comprehensive test suite for boundary constraint scenarios
✅ Added test cases for edge cases (left/right of boundary, at min/max boundaries, within boundaries)

Type of Change

  • ✨ New feature
  • 🐛 Bug fix
  • 💥 Breaking change
  • 📖 Documentation

🧪 Testing

✅ Unit tests added for boundary constraint validation
✅ Test cases for LineIndicator left of boundary (should not render)
✅ Test cases for LineIndicator at min boundary (should render)
✅ Test cases for LineIndicator within boundary (should render)
✅ Test cases for LineIndicator at max boundary (should render)
✅ Test cases for LineIndicator right of boundary (should not render)
✅ All tests passing
✅ Manual testing performed with mouse cursor interactions

📦 Developer Experience Improvements

  • Improved code maintainability through better coordinate management
  • Consistent boundary validation across chart interactions
  • Reduced potential for visual overflow issues
  • Enhanced reliability of line indicator positioning

⚠️ Breaking Changes

  • None

Author: Miguel García (@miguelgarglez)

- Add boundary validation to prevent LineIndicator from exceeding xAxis coordinates
- Implement isCursorWithinPathArea check to only render indicator within data area
- Update y-axis coordinate calculation to use pre-calculated axis values
- Add comprehensive tests for boundary constraint scenarios

This fix addresses issues where the line indicator could extend beyond the
chart boundaries into the axis labels area.
@vercel
Copy link
Copy Markdown

vercel bot commented Dec 10, 2025

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

Project Deployment Preview Comments Updated (UTC)
kubit-react-charts Ready Ready Preview Comment Dec 10, 2025 4:59pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 10, 2025

🔍 PR Validation Report

Status:PASSED
Branch: bugfix/line-indicator-limit
Expected Version Bump: patch


✅ Validation Results

Check Status Description
Branch Naming ✅ Pass Must follow type/description pattern
PR Title ✅ Pass Must follow conventional commits
ESLint ✅ Pass Code style and quality rules
TypeScript ✅ Pass Type checking validation
Tests ✅ Pass All tests must pass
Code Quality ✅ Pass No console.log, TODOs need issues
Documentation ✅ Pass Updated for new features

🎯 Next Steps

This PR is ready for review - all validation checks passed!

📦 Expected Release Impact

Based on branch name bugfix/line-indicator-limit, this PR will trigger a patch version bump when merged.


This validation runs automatically on every PR. Questions? Check our Contributing Guidelines

@hectoruch hectoruch merged commit b3b7c5f into cms Dec 11, 2025
4 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

❌ Auto-publish Failed

The automatic publication process failed. Please check the workflow logs for details.

🔧 Common Solutions

  • NPM Token: Verify NPM_TOKEN is valid and has publish permissions
  • Release Token: Add RELEASE_TOKEN secret to bypass branch protection rules
  • Token Permissions: Check that tokens have correct permissions
  • Version Conflict: Check if version already exists in NPM
  • Build Issues: Ensure all tests pass locally and build completes successfully

🔐 Required Secrets Configuration

  1. NPM_TOKEN:

    • Type: "Automation" token from npmjs.com
    • Scope: Access to publish the package
  2. RELEASE_TOKEN (Required for protected branches):

    • Type: Personal Access Token with bypass permissions
    • Permissions: "Contents: write", "Pull requests: write"
    • Special: "Bypass pull request requirements" if needed
  3. CHROMATIC_PROJECT_TOKEN:

    • Type: Project token from chromatic.com
    • Scope: Access to publish builds and visual regression tests

📞 Next Steps

  1. NPM Issues: Verify NPM_TOKEN is an automation token
  2. Branch Protection: Add RELEASE_TOKEN secret with bypass permissions
  3. Logs: Check error logs for specific authentication issues
  4. Manual Process: Create a new PR if tokens can't be configured

@github-actions
Copy link
Copy Markdown
Contributor

🐛 Auto-publish Successful!

Field Value
Branch bugfix/line-indicator-limit
Type patch
Version 1.5.01.5.1
NPM @kubit-ui-web/react-charts@1.5.1

📦 Installation

npm install @kubit-ui-web/react-charts@1.5.1
# or
yarn add @kubit-ui-web/react-charts@1.5.1

✅ Completed Steps

  • Quality checks passed
  • Tests passed
  • Build successful
  • Storybook built for Chromatic
  • Published to Chromatic
  • Published to NPM
  • GitHub release created
  • Repository tagged

🎉 Ready to use in production!

@hectoruch hectoruch deleted the bugfix/line-indicator-limit branch February 4, 2026 11:16
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.

3 participants