[google_maps_flutter_web] Fix AdvancedMarker anchors on web#11966
Conversation
|
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 implements AdvancedMarker anchor handling for the Google Maps Flutter Web plugin. It introduces helper functions to convert anchor offsets to CSS percentage strings and apply them as anchorLeft and anchorTop properties on the underlying AdvancedMarkerElement and its options. Additionally, an integration test has been added to verify that anchors are correctly set and updated. I have no feedback to provide.
There was a problem hiding this comment.
Code Review
This pull request fixes the anchor handling for AdvancedMarker on the web platform by introducing helper functions to convert anchor offsets to CSS percentages and apply them to AdvancedMarkerElementOptions and AdvancedMarkerElement. It also adds an integration test to verify anchor positioning and updates, and bumps the package version to 0.6.2+4. There are no review comments, and I have no feedback to provide.
|
Thanks for the contribution! There are two email addresses associated with the commits in this PR, and only one has a CLA on file. You'll need to either sign the CLA for the second address, or if using it was unintentional, amend your commits and re-push. Once that's done, please mark as Read for Review. |
There was a problem hiding this comment.
Code Review
This pull request implements AdvancedMarker anchor handling on the web platform by converting anchor offsets to CSS percentage strings and applying them to the marker options, supported by new integration tests. Feedback suggests addressing potential floating-point precision issues in the percentage calculation to prevent verbose CSS output.
| String _advancedMarkerAnchorToCssOffset(double anchor) { | ||
| final double percentage = -anchor * 100; | ||
| if (percentage == percentage.roundToDouble()) { | ||
| return '${percentage.toInt()}%'; | ||
| } | ||
| return '$percentage%'; | ||
| } |
There was a problem hiding this comment.
Floating-point multiplication (e.g., -anchor * 100) can sometimes introduce precision artifacts (for example, -0.14 * 100 results in -14.000000000000002 in Dart/JS). This can cause the strict equality check percentage == percentage.roundToDouble() to fail, leading to unnecessarily verbose CSS percentage strings like '-14.000000000000002%' instead of '-14%'.
Using a small epsilon check when comparing the percentage to its rounded integer value will make the output cleaner and more robust against floating-point precision issues.
| String _advancedMarkerAnchorToCssOffset(double anchor) { | |
| final double percentage = -anchor * 100; | |
| if (percentage == percentage.roundToDouble()) { | |
| return '${percentage.toInt()}%'; | |
| } | |
| return '$percentage%'; | |
| } | |
| String _advancedMarkerAnchorToCssOffset(double anchor) { | |
| final double percentage = -anchor * 100; | |
| final int rounded = percentage.round(); | |
| if ((percentage - rounded).abs() < 1e-9) { | |
| return '$rounded%'; | |
| } | |
| return '$percentage%'; | |
| } |
| import 'dart:async'; | ||
| import 'dart:convert'; | ||
| import 'dart:js_interop'; | ||
| import 'dart:js_interop_unsafe'; |
There was a problem hiding this comment.
Why is this using unsafe dynamic lookup? If there are properties that aren't reflected in google_maps, they should be added upstream, rather than bypassing the wrapper.
Fixes AdvancedMarker anchor handling in
google_maps_flutter_web.Previously
AdvancedMarker.anchorwas not applied on web, so custom advanced marker content was positioned as if it used the default anchor. This caused markers with non-default anchors to appear offset from their intended map coordinates.This PR converts Flutter marker anchor offsets into the Google Maps JavaScript Advanced Marker
anchorLeftandanchorTopCSS offset properties when creating marker options, and reapplies those properties when an existing advanced marker is updated.Adds integration test coverage for creating and updating an
AdvancedMarkerwith custom anchors.Addresses flutter/flutter#80578.
Pre-Review Checklist
[shared_preferences]///).