Fix PieChartEvent.type on web
#3611
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request addresses issue #3546 by improving the event handling in the PieChart component for web. It introduces a mapping for Flutter event types to string representations and includes local position data in the event payload. Additionally, minor adjustments were made to the Theme and ScrollableControl classes for better type handling and code cleanup. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ndonkoHenri - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
Comment on lines
+18
to
+27
| const eventMap = { | ||
| "FlPointerEnterEvent": "pointerEnter", | ||
| "FlPointerExitEvent": "pointerExit", | ||
| "FlPointerHoverEvent": "pointerHover", | ||
| "FlPanCancelEvent": "panCancel", | ||
| "FlPanDownEvent": "panDown", | ||
| "FlPanEndEvent": "panEnd", | ||
| "FlPanStartEvent": "panStart", | ||
| "FlPanUpdateEvent": "panUpdate", | ||
| "FlLongPressEnd": "longPressEnd", |
There was a problem hiding this comment.
suggestion: Consider making eventMap private
If eventMap is only used within this file, consider prefixing it with an underscore to indicate that it is private and should not be accessed from outside this module.
|
Can you resolve conflict here please? |
# Conflicts: # sdk/python/packages/flet-core/src/flet_core/charts/pie_chart.py # sdk/python/packages/flet-core/src/flet_core/scrollable_control.py
FeodorFitsner
approved these changes
Jul 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3546
Tried another approach, but the issue is still present.
AI's opinion on the cause of the issue:
What I found strange is the fact that, when testing on web using
flutter run -d chromeit works (shows the right types).Summary by Sourcery
This pull request addresses the issue with
PieChartEvent.typeon web by mapping Flutter event types to readable event names. It also enhances the event data by adding local position information and introduces an enum for event types in the Python SDK.PieChartEvent.typeon web by mapping Flutter event types to readable event names.localPositiontoPieChartEventDatato capture the local position of events.PieChartEventTypeenum in Python to standardize event type values.