[google_maps_flutter_platform_interface] Add editable polyline and polygon API#11492
[google_maps_flutter_platform_interface] Add editable polyline and polygon API#11492lume-code wants to merge 1 commit into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for user-editable Polygons and Polylines on the web platform by adding an editable property and an onEdited callback to the respective classes. The implementation includes new event types in the platform interface and logic in the web plugin to listen for path changes via the Google Maps JavaScript API. Review feedback highlights that the current Polygon implementation fails to include hole data in the edit events and callbacks. Furthermore, the web controllers only set up edit listeners during construction, which prevents the functionality from working if the editable property is enabled after the object is created.
d90a83b to
00e1ec3
Compare
This PR has substantive changes to three packages; so can't have comprehensive test coverage without adding tests in all three. |
|
From triage: @lume-code Are you planning on adding the necessary test coverage for this to move forward? |
|
From web triage: @lume-code are you still planning to move forward with this PR? |
648f183 to
81bf57a
Compare
|
@mdebbar yes, still moving forward — sorry for the delay. Web test coverage is now in (commit 81bf57a), and the branch has been rebased onto current The new tests live under
All tests pass locally. Ready for another look whenever you have a moment. |
mdebbar
left a comment
There was a problem hiding this comment.
Changes look good to me!
I think this PR now needs to be split so the platform interface package lands first?
…n API Adds the platform-interface surface for editable polylines and polygons: - `editable` property and `onEdited` callback on `Polyline` and `Polygon`. - `PolylineEditEvent` and `PolygonEditEvent` map event types. - `onPolylineEdited` / `onPolygonEdited` streams on the platform interface (default `UnimplementedError`, so the addition is non-breaking). Bumps to 2.16.0. First of three PRs splitting flutter#11492 along the federated plugin (interface lands and publishes first).
81bf57a to
965e08d
Compare
|
Thanks for the review! As suggested, I've split this along the federated plugin so the platform interface can land and publish first. This PR (#11492) is now scoped to
|
Implements editable polylines and polygons on web using the Google Maps JavaScript API `editable` flag: - Wires `editable`/`onEdited` through to the JS `Polyline`/`Polygon` controllers. - Listens to MVCArray path-change events and emits `PolylineEditEvent` / `PolygonEditEvent` (including polygon holes). - Recreates the controller on change so edit listeners rewire when `editable` toggles. Bumps to 0.7.0. Second of three PRs splitting flutter#11492; depends on google_maps_flutter_platform_interface 2.16.0.
Subscribes to the platform interface `onPolylineEdited` / `onPolygonEdited` streams and dispatches them to the `onEdited` callbacks on `Polyline` / `Polygon`. Editing is currently only supported on web. Bumps to 2.18.0. Third of three PRs splitting flutter#11492; depends on google_maps_flutter_platform_interface 2.16.0 and google_maps_flutter_web 0.7.0.
Implements editable polylines and polygons on web using the Google Maps JavaScript API `editable` flag: - Wires `editable`/`onEdited` through to the JS `Polyline`/`Polygon` controllers. - Listens to MVCArray path-change events and emits `PolylineEditEvent` / `PolygonEditEvent` (including polygon holes). - Recreates the controller on change so edit listeners rewire when `editable` toggles. Bumps to 0.7.0. Second of three PRs splitting flutter#11492; depends on google_maps_flutter_platform_interface 2.16.0.
Adds the platform-interface API surface for editable polylines and polygons. This has been requested since November 2020 (flutter/flutter#71248).
Changes —
google_maps_flutter_platform_interface(2.15.0 → 2.16.0)editable(bool, defaultfalse) andonEdited(callback) fields toPolylineandPolygon.PolylineEditEventandPolygonEditEventmap event types.onPolylineEditedandonPolygonEditedstreams toGoogleMapsFlutterPlatform(defaultUnimplementedError).All additions are non-breaking: the new constructor parameters are defaulted/optional, and the new platform methods have default implementations.
Editing is implemented for web only (see #11919); the property is silently ignored on Android and iOS.
Fixes flutter/flutter#71248
Pre-Review Checklist
[shared_preferences]///).Footnotes
See the test exemption and version/CHANGELOG exemption docs for details. ↩ ↩2