-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Both scaleable and draggable chart implemented #1728
base: main
Are you sure you want to change the base?
Conversation
Would be great if this could be merged for the next implementation. |
|
||
R? response; | ||
if (pointerCount > 1) { | ||
_touchCallback!(event, response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just set a null as the second parameter (instead of having a nullable R?, which is always null)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix.
_scaleGestureRecognizer | ||
..onStart = (details) { | ||
_notifyScaleEvent(FlScaleStartEvent(details)); | ||
_notifyTouchEvent(FlPanDownEvent(details)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay to call both FlPanDown and FlPanStart at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not, drag down details and scalestart details will be different and be a breaking change. Thinking of another solution
_scaleGestureRecognizer = ScaleGestureRecognizer(); | ||
_scaleGestureRecognizer | ||
..onStart = (details) { | ||
_notifyScaleEvent(FlScaleStartEvent(details)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it get called for the second finger (when you already have your first finger, and try to add the second finger to do the scale gesture)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I think we should have something like fingerIndex or something like that
_notifyTouchEvent(FlPanStartEvent(dragStartDetails)); | ||
..onUpdate = (details) { | ||
_notifyScaleEvent(FlScaleUpdateEvent(details)); | ||
_notifyTouchEvent(FlPanUpdateEvent(details)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also for here, (something like fingerIndex)
Hi @adamsocrat @imaNNeo , joining the conversation here. I think it might be a bit inconsistent with the rest of the flutter framework to raise scale events for 2 fingers and pan for 1 finger. To my limited understanding, the idea behind this is that - scale is also a pan. with the scale events, you could implement pan because it already has all the necessary information. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1728 +/- ##
=======================================
Coverage 89.17% 89.17%
=======================================
Files 44 44
Lines 3031 3031
=======================================
Hits 2703 2703
Misses 328 328 ☔ View full report in Codecov by Sentry. |
Implements #1720
I used the idea from #1721 and implemented FlScaleEvent but without explicitly specifying
detectScale
to disable drag fully when scaling.detectScale
could've been set dynamically but it was combursome and couldn't get it working. Implementing a new FLTouchEvent seemed better option.The drawback was as in #1721 mentioned
PanGestureRecognizer
andScaleGestureRecognizer
does not like to be used together. ButScaleGestureRecognizer
event details also feeds thePanGestureRecognizer
event details (except onDown) see, so instead of disabling dragging fully when you want to scale, I notify ScaleEvent when there are two points and drag when one point is in contact with the screen. Same idea of this comment! Details feeding into
FlPan...Event
have been switched to Scale details. Testing may be needed.You can get TouchEvent from touchcallback as
with the
FlScaleEvent
I am able to setminX
andmaxX
to zoom in/out chart without the scale is being also interpreted as drag. See #71