Skip to content

Conversation

@FeodorFitsner
Copy link
Contributor

@FeodorFitsner FeodorFitsner commented Oct 23, 2025

Updates diffing and protocol encoding logic to distinguish 'on_' fields that are not events using metadata. Adds integration tests for color scheme, new test cases for 'on_' fields, and updates ColorScheme to mark non-event 'on_' fields. This improves accuracy of change detection and serialization for controls with 'on_' prefixed fields.

Summary by Sourcery

Distinguish non-event on_* fields in controls by using metadata and update diffing, protocol encoding, and attribute assignment logic accordingly, with new tests to validate behavior

Enhancements:

  • Mark ColorScheme on_* fields as non-events via metadata to prevent incorrect event handling
  • Restrict diff and protocol encoding to boolean-convert only on_* fields marked as events
  • Simplify attribute setting logic by removing special-case conversion for on_* fields

Tests:

  • Add unit tests for object diffing non-event on_* fields in frozen and in-place scenarios
  • Add integration test for ColorScheme rendering with on_* color values

Updates diffing and protocol encoding logic to distinguish 'on_' fields that are not events using metadata. Adds integration tests for color scheme, new test cases for 'on_' fields, and updates ColorScheme to mark non-event 'on_' fields. This improves accuracy of change detection and serialization for controls with 'on_' prefixed fields.
@FeodorFitsner FeodorFitsner changed the title Handle non-event 'on_' fields in diff and protocol logic Handle non-event on_ fields in diff and protocol logic Oct 23, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a 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

Copy link
Contributor

Copilot AI left a 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 enhances the handling of fields prefixed with on_ to distinguish between event handlers and regular data fields (like color properties). The changes add metadata-based classification using {"event": False} to mark non-event on_ fields, ensuring they are properly serialized and tracked during diff operations instead of being treated as boolean event indicators.

Key changes:

  • Added metadata to ColorScheme's on_* color fields to mark them as non-events
  • Updated protocol encoding and diff logic to check field metadata before treating on_ fields as events
  • Added comprehensive test coverage for on_ field handling in both frozen and in-place diff scenarios

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
sdk/python/packages/flet/src/flet/controls/theme.py Marked all on_* color fields in ColorScheme with {"event": False} metadata
sdk/python/packages/flet/src/flet/messaging/protocol.py Updated encoding logic to check metadata before treating on_ fields as events
sdk/python/packages/flet/src/flet/controls/object_patch.py Updated diff comparison logic to check metadata for on_ fields
sdk/python/packages/flet/tests/test_object_diff_in_place.py Added test for on_ field handling in in-place diff
sdk/python/packages/flet/tests/test_object_diff_frozen.py Added tests for on_ field handling in frozen diff scenarios
sdk/python/packages/flet/tests/common.py Extended MyText test control with color_scheme and on_select fields
sdk/python/packages/flet/integration_tests/controls/theme/test_color_scheme.py Added integration test for ColorScheme functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Oct 23, 2025

Deploying flet-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8e8dbdc
Status: ✅  Deploy successful!
Preview URL: https://301183d8.flet-docs.pages.dev
Branch Preview URL: https://fields-with-on-fix.flet-docs.pages.dev

View logs

Updated logic in material Jinja templates to more accurately classify event attributes by checking config.extra.events. Also updated colorscheme.md to pass additional options for documentation rendering.
Simplifies checks for 'event' metadata in object_patch.py and protocol.py by using get() with a default value. Updates color scheme integration test to use a more comprehensive set of ColorScheme properties and renames screenshot references for clarity.
Added wrapIntoScrollableView option to ScrollableControl and enabled it for Column, Row, and View controls to ensure proper scroll behavior. Enhanced ListViewControl to handle spacing and dividers between children more flexibly. Refined Scrollbar visibility logic and removed unnecessary exception in ViewControl.
Expanded the color scheme integration test to cover palettes, surface roles, accents, buttons, themed card, and error banner. Replaced the single color_scheme.png screenshot with multiple targeted screenshots and updated the test logic and golden images accordingly.
Renamed 'dismissable_list_tiles.py' and its media file to 'dismissible_list_tiles.py' for consistency. Added a new example 'remove_on_dismiss_declarative.py' demonstrating proper use of keys with Dismissible in declarative components. Updated documentation to reflect these changes and provide guidance on key usage.
Enhanced logic for determining the prototype item in ListViewControl (Dart) to use either a provided prototype or the first control when appropriate. Updated Python docstrings to clarify when prototype_item and first_item_prototype properties take effect.
Comment on lines 57 to 59
Note: This property has effect only when [`build_controls_on_demand`][(c).]
is `True` or [`spacing`][(c).] is `0`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

"Note:" must be alone one one line.

Note:
    text here...

Limits macOS integration tests to the 'controls/theme' suite for focused testing. Improves docstring formatting in ListView by splitting 'Note:' onto its own line for better readability.
Reorganized color palette and button rows in test_color_scheme.py, added ScrollKey to palette rows, removed redundant spacing and text labels, and adjusted error banner border radius. Updated corresponding golden images to reflect UI changes for macOS color scheme integration tests.
Regenerated golden images for macOS color scheme integration tests and adjusted test window height from 600 to 300 to match new screenshot dimensions.
Updated the color scheme integration test to wrap palette, button, card, and banner controls in Screenshot widgets for more precise screenshot capturing. Adjusted window height and consolidated button rows. Updated screenshot assertions to use the new capture method for each control.
Restores the full test suite matrix in the macOS integration tests workflow, running tests for apps, examples, and all controls. Previously, only the 'controls/theme' suite was enabled.
Corrected a typo in the Marker class docstring in marker_layer.py and updated the filter regex in mkdocs.yml to properly match method names for documentation exclusion.
@FeodorFitsner FeodorFitsner merged commit 42dcc71 into main Oct 25, 2025
23 of 37 checks passed
@FeodorFitsner FeodorFitsner deleted the fields-with-on-fix branch October 25, 2025 21:21
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