[google_maps_flutter_android] Add PlatformCap pigeon type.
#7639
Conversation
yaakovschectman
requested review from
reidbaker and
stuartmorgan
as code owners
| /// Converts platform interface's Cap to Pigeon's PlatformCap. | ||
| @visibleForTesting | ||
| PlatformCap platformCapFromCap(Cap cap) { | ||
| final List<Object> json = cap.toJson() as List<Object>; |
There was a problem hiding this comment.
My comment here applies to this as well (as does Reid's comment about needing to document the source of what's being interpreted).
It would be fine to use a single issue to cover both, since it's basically the same issue, and we could fix both at once pretty easily.
| PlatformCap({required this.type, this.bitmapDescriptor, this.refWidth}); | ||
|
|
||
| final PlatformCapType type; | ||
| final Object? bitmapDescriptor; |
There was a problem hiding this comment.
This needs a version of the same comment that you removed for startCap/endCap, and a tracking issue. (It looks like this will be another annoying one like CameraUpdate.)
| @@ -2102,6 +2110,124 @@ ArrayList<Object> toList() { | |||
| } | |||
| } | |||
|
|
|||
| /** | |||
| * Pigeon equivalent of the Cap class. | |||
There was a problem hiding this comment.
Instead of saying "the Cap class" consider linking to the source of that class.
I believe this is the correct class. https://developers.google.com/maps/documentation/android-sdk/reference/com/google/android/libraries/maps/model/Cap
There was a problem hiding this comment.
Should this link to google map's SDK's Cap? I would have thought a link to the platform interface's Cap would be more appropriate, but do you disagree?
There was a problem hiding this comment.
I think the fact that you and I were thinking of different classes is a great explanation of why "the cap class" is not the right level of documentation.
There was a problem hiding this comment.
I've linked here to the platform interface Cap, and from a couple other comments in this PR to the Maps SDK's Cap. Let me know if you agree with my choice of links.
| PlatformJointType jointType; | ||
|
|
||
| /// The pattern data, as JSON. Each element in this list should be set only from PatternItem.toJson, and the native code must interpret it according to the internal implementation details of that method. | ||
| List<Object?> patterns; | ||
|
|
||
| List<PlatformLatLng?> points; | ||
|
|
||
| /// The start and end cap data, as JSON. These should be set only from Cap.toJson, and the native code must interpret it according to the internal implementation details of that method. | ||
| Object startCap; | ||
| /// The start and end cap data. |
There was a problem hiding this comment.
Link to documentation so that someone can better understand these values.
| @@ -1166,6 +1166,37 @@ PlatformJointType platformJointTypeFromJointType(JointType jointType) { | |||
| return PlatformJointType.mitered; | |||
| } | |||
|
|
|||
| /// Converts platform interface's Cap to Pigeon's PlatformCap. | |||
There was a problem hiding this comment.
No action required: You are following the pattern of the rest of this file but both this method name and documentation do not help me understand what is being converted here or how to learn about either the to or from object.
| final bool visible; | ||
| final int width; | ||
| final int zIndex; | ||
| } | ||
|
|
||
| /// Enumeration of possible types of Cap. |
There was a problem hiding this comment.
Link to cap documentation.
Replace old JSON implementation of polyline start/end cap with a structured Pigeon.
flutter/flutter#154737
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.