Skip to content

feat: Recognize secondary button pointer events#1989

Merged
imaNNeo merged 3 commits intoimaNNeo:mainfrom
enwi:main
Apr 15, 2026
Merged

feat: Recognize secondary button pointer events#1989
imaNNeo merged 3 commits intoimaNNeo:mainfrom
enwi:main

Conversation

@enwi
Copy link
Copy Markdown
Contributor

@enwi enwi commented Sep 5, 2025

This pull request adds support for distinguishing between primary and secondary button interactions in chart touch events. The main changes involve creating special secondary event classes and wiring up the appropriate gesture recognizer callbacks to handle secondary button events.

Touch event model enhancements:

  • Added secondary touch event classes (FlTapDownSecondaryEvent, FlTapCancelSecondaryEvent, FlTapUpSecondaryEvent, FlLongPressSecondaryStart, FlLongPressMoveSecondaryUpdate, FlLongPressSecondaryEnd) in fl_touch_event.dart to indicate if the event was triggered by a secondary button.

Gesture recognizer integration:

  • Updated render_base_chart.dart to handle secondary tap and long press gestures by wiring up the corresponding gesture recognizer callbacks.

These changes enable the chart to differentiate between primary and secondary pointer interactions, which is useful for supporting right-click or other alternate input methods.

@imaNNeo
Copy link
Copy Markdown
Owner

imaNNeo commented Oct 25, 2025

Hi Owen,
Thanks for your contribution.

I think it's better to define different event types for the secondary events (instead of adding a flag inside the current events).
And the reason is the fact that it affects the current logic that users have. Suppose that they implemented a logic that only accepts the primary tap. After updating to the new version, they also support the secondary tap. (I know they can update their logic to check if it is a primary tap or not, but that's not backward compatible)

So I just suggest defining new event types for secondary events, then anyone can support the secondary button, and the current implementation remains untouched.

Thanks!

@enwi
Copy link
Copy Markdown
Contributor Author

enwi commented Oct 27, 2025

Hey @imaNNeo I have updated the PR as you suggested 🙂

@imaNNeo
Copy link
Copy Markdown
Owner

imaNNeo commented Apr 10, 2026

Perfect! Thanks for implementing the new approach.
It looks good to me, but before merging, there are a couple of things to mention:

  • To keep the consistency with the tap callbacks in Flutter itself, I suggest using the Secondary keyword as a prefix, instead of a postfix. For example:
    • FlTapDownSecondaryEvent -> FlSecondaryTapDownEvent
    • FlTapCancelSecondaryEvent -> FlSecondaryTapCancelEvent
    • FlLongPressSecondaryStart -> FlSecondaryLongPressStart
    • FlLongPressMoveSecondaryUpdate -> FlSecondaryLongPressMoveUpdate
    • ...
  • Can you please update the documentation in base_chart.md?
  • Is there any possibility of writing a unit test for it? (I know it is deep and low-level. But there might be a way to test that

@enwi enwi changed the title Recognize secondary button pointer events Feat: Recognize secondary button pointer events Apr 13, 2026
@enwi enwi changed the title Feat: Recognize secondary button pointer events feat: Recognize secondary button pointer events Apr 13, 2026
@enwi
Copy link
Copy Markdown
Contributor Author

enwi commented Apr 13, 2026

@imaNNeo I have updated the code as you suggested. As there do not exist any tests for other events currently I am not sure if I find the time to write them.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.00%. Comparing base (c83aa90) to head (b3819ff).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1989   +/-   ##
=======================================
  Coverage   93.00%   93.00%           
=======================================
  Files          50       50           
  Lines        3917     3917           
=======================================
  Hits         3643     3643           
  Misses        274      274           
Flag Coverage Δ
flutter 93.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@imaNNeo
Copy link
Copy Markdown
Owner

imaNNeo commented Apr 15, 2026

Thanks for your contribution!

@imaNNeo imaNNeo merged commit 0a2dcac into imaNNeo:main Apr 15, 2026
5 of 7 checks passed
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.

2 participants