-
Notifications
You must be signed in to change notification settings - Fork 590
fix(flet-charts): Refactor chart event parser to overcome minification #5797
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
Conversation
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.
We've reviewed this pull request using the Sourcery rules engine
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.
Pull Request Overview
This PR refactors event type resolution across all chart components by replacing a runtime type string lookup with direct type checking. The change improves type safety and performance by using Dart's type system instead of string-based runtime type resolution.
Key Changes:
- Replaced the
eventMapconstant lookup with a newresolveFlTouchEventType()function that usesistype checks - Updated all chart event data factories (bar, candlestick, line, pie, radar, scatter) to use the new function
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/charts.dart |
Removed eventMap constant and added resolveFlTouchEventType() function using type checks instead of string lookup |
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/bar_chart.dart |
Updated BarChartEventData.fromDetails() to call resolveFlTouchEventType() |
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/candlestick_chart.dart |
Updated CandlestickChartEventData.fromDetails() to call resolveFlTouchEventType() |
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/line_chart.dart |
Updated LineChartEventData.fromDetails() to call resolveFlTouchEventType() |
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/pie_chart.dart |
Updated PieChartEventData.fromDetails() to call resolveFlTouchEventType() |
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/radar_chart.dart |
Updated RadarChartEventData.fromDetails() to call resolveFlTouchEventType() |
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/scatter_chart.dart |
Updated ScatterChartEventData.fromDetails() to call resolveFlTouchEventType() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/bar_chart.dart
Outdated
Show resolved
Hide resolved
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/candlestick_chart.dart
Outdated
Show resolved
Hide resolved
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/line_chart.dart
Show resolved
Hide resolved
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/pie_chart.dart
Outdated
Show resolved
Hide resolved
sdk/python/packages/flet-charts/src/flutter/flet_charts/lib/src/utils/radar_chart.dart
Outdated
Show resolved
Hide resolved
Corrected indentation for eventType parameter in the constructors of BarChartEventData, CandlestickChartEventData, PieChartEventData, and RadarChartEventData for improved code readability and consistency.
Fix #5219
Continuation of #3611
Summary by Sourcery
Refactor chart event parsing for flet-charts to use a type-based resolver function instead of a string-keyed map, ensuring event types are correctly identified in minified builds.
Bug Fixes:
Enhancements:
Chores: