Skip to content

Fix Touch Issue#176

Open
Fifteen15Studios wants to merge 1 commit intohi-manshu:mainfrom
Fifteen15Studios:patch-1
Open

Fix Touch Issue#176
Fifteen15Studios wants to merge 1 commit intohi-manshu:mainfrom
Fifteen15Studios:patch-1

Conversation

@Fifteen15Studios
Copy link

@Fifteen15Studios Fifteen15Studios commented Jan 31, 2026

Fix issue where touching the first item in a pie chart does not always register properly.

Summary by CodeRabbit

  • Bug Fixes
    • Improved click-detection accuracy for pie charts, particularly at angle boundaries.

✏️ Tip: You can customize this high-level summary in your review settings.

Fix issue where touching the first item in a pie chart does not always register properly.
@coderabbitai
Copy link

coderabbitai bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

The PieChart click-detection logic is enhanced by adding modulo 360 normalization to two angles—touchAngle and normalizedAngle—to properly handle angle wrap-around edge cases during user interaction detection.

Changes

Cohort / File(s) Summary
PieChart Click-Detection
charty/src/commonMain/kotlin/com/himanshoe/charty/pie/PieChart.kt
Added modulo 360 normalization to touchAngle and normalizedAngle in click-detection logic to handle angle wrap-around edge cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🥕 A rabbit's note on angles true,
Spinning 'round at 360 we go,
Modulo keeps clicks right on cue,
No wraparound steals the show! 🥧

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix Touch Issue' is vague and generic. While it relates to the changeset (touch detection in PieChart), it lacks specificity about what the actual fix is—the modulo 360 normalization for angle calculations. Consider a more descriptive title such as 'Fix pie chart touch detection with angle normalization' or 'Normalize angles in pie chart click detection to fix touch registration issues'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@Fifteen15Studios
Copy link
Author

This can be a little prettier, and use your constant, by using the following lines instead:

else touchAngle %= FULL_CIRCLE_DEGREES

and

else normalizedAngle %= FULL_CIRCLE_DEGREES

And technically normalizedAngle is the one the fixes the issue, but both should be properly adjusted to be below 360.

@Fifteen15Studios
Copy link
Author

This bug is caused when config.startAngleDegrees is negative, such as the default of -90f, and the user clicks somewhere in the top right quarter of the circle. (Less than config.startAngleDegrees from the X axis.) When this happens, the touch angle is between 270 and 360, then normalizedAngle is set to touchAngle - config.startAngleDegrees which is more than 360 degrees.

Example: config.startAngleDegrees = -90f and Touch is at 280 degrees.

touchAngle - config.startAngleDegrees becomes 280 -(-90) or 280 + 90 which is 370.

When the comparison is made of normalizedAngle < currentAngle + sweepAngle, normalizedAngle is always going to be larger, since it is more than a full circular rotation, so this can never be true. By using mod 360, we change normalizedAngle from 370 to 10, which satisfies this comparison.

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