-
Notifications
You must be signed in to change notification settings - Fork 596
refactor!: consolidate src_base64, src, and src_bytes into unified/intelligent src prop
#5817
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
base: main
Are you sure you want to change the base?
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.
Sorry @ndonkoHenri, your pull request is larger than the review limit of 150000 diff characters
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 consolidates three separate properties (src, src_base64, src_bytes) into a single intelligent src property that can automatically detect and handle URLs, file paths, base64-encoded strings, and raw bytes. This is a breaking change that simplifies the API across multiple controls (Image, Lottie, Audio, CircleAvatar, DecorationImage, Canvas.Image).
Key changes:
- Python API now uses
Union[str, bytes]for thesrcproperty - Dart implementation uses
ResolvedAssetSourceutility to intelligently parse the source - Documentation and examples updated to reflect the unified API
- Tests updated to use the new consolidated property
Reviewed Changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
circle_avatar.py |
Changed foreground_image_src and background_image_src to accept Union[str, bytes] |
bottom_sheet.py |
Documentation improvement (unrelated to main change) |
image.py |
Removed src_base64 and src_bytes, consolidated into src: Union[str, bytes] |
canvas/image.py |
Removed src_bytes, updated src to Optional[Union[str, bytes]] |
box.py |
Removed src_base64 and src_bytes from DecorationImage, updated copy method |
lottie.dart |
Uses ResolvedAssetSource instead of separate base64 handling |
lottie.py |
Removed src_base64, changed default for enable_merge_paths to True |
audio.dart |
Uses ResolvedAssetSource with listEquals for bytes comparison |
audio.py |
Removed src_base64, consolidated into src: Optional[Union[str, bytes]] |
strings.dart |
Added isBase64 and stripBase64DataHeader() extension methods |
| Various test files | Updated to use consolidated src property |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
|
|
||
| src: Optional[str] = None | ||
| src: Union[str, bytes] |
Copilot
AI
Nov 15, 2025
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.
The src property should be Optional[Union[str, bytes]] instead of requiring it, as there may be cases where error_content needs to be shown without a source. Looking at other controls like canvas/image.py which uses Optional[Union[str, bytes]], this should be consistent.
| """ | ||
|
|
||
| src: Optional[str] = None | ||
| src: Union[str, bytes] |
Copilot
AI
Nov 15, 2025
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.
Similar to Image control, the src property should be Optional[Union[str, bytes]] for consistency and to allow for optional rendering with error_content fallback.
| src: Union[str, bytes] | |
| src: Optional[Union[str, bytes]] |
| bool get isBase64 { | ||
| var s = stripBase64DataHeader().replaceAll(RegExp(r'\s+'), ''); | ||
|
|
||
| try { | ||
| final normalized = base64.normalize(s); | ||
| base64.decode(normalized); | ||
| return true; | ||
| } catch (_) { | ||
| return false; | ||
| } | ||
| } |
Copilot
AI
Nov 15, 2025
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.
The isBase64 getter performs two operations: normalization and decoding. If this is called frequently, consider caching the result or avoiding the decode step if normalization is sufficient for validation. The decode operation allocates memory that's immediately discarded.
| int arrayIndexOf(Uint8List haystack, Uint8List needle) { | ||
| var len = needle.length; | ||
| var limit = haystack.length - len; | ||
| final len = needle.length; | ||
| if (len == 0) return 0; |
Copilot
AI
Nov 15, 2025
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.
[nitpick] The edge case handling for empty needle returns 0, which could be confusing. Consider documenting why empty needle returns 0 (typically means 'found at start') or consider returning -1 for consistency with 'not found' semantics.
| } else if (resolvedSrc.bytes != null && | ||
| (_srcBytes == null || !listEquals(_srcBytes, resolvedSrc.bytes))) { |
Copilot
AI
Nov 15, 2025
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.
Using listEquals for comparing potentially large audio byte arrays on every update could be expensive. Consider using a hash or length check as a quick first pass before doing full byte comparison, or storing a hash of the bytes for efficient comparison.
Deploying flet-docs with
|
| Latest commit: |
a31b0a0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f8ddf9f9.flet-docs.pages.dev |
| Branch Preview URL: | https://unified-src.flet-docs.pages.dev |
# Conflicts: # sdk/python/packages/flet/integration_tests/controls/core/test_image.py
Fix #5470
Example: