[google_maps_flutter] Add pointForMeters to plugin in iOS #2503

klaes-ashford · 2020-02-05T21:18:15Z

Description

This PR will implement the pointForMeters projection api method in the plugin for iOS. This method helps you get the distance in meters to content size from the specified LatLng in the map.

iOS maps sdk doesn't support drawing dotted polylines out of the box. Instead we make use of drawing circles between two LatLngs. We have to make use of pointForMeters api method to draw circles without specifying a constant radius so that the circle scales regardless of zoom.

Related Issues

flutter/flutter#37959
#2108

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

klaes-ashford · 2020-02-07T02:50:14Z

@cyanglaz drawing dotted polylines in iOS makes use of this api since SDK doesn't support it out of the box. I can't find android equivalent for the same as the map sdk for android doesn't seem to expose this method. What should be done in this case ?

cyanglaz

Thanks for the PR. I left some comments.
Is there a way to add Android implementation?

packages/google_maps_flutter/example/test_driver/google_maps_e2e.dart

packages/google_maps_flutter/lib/src/controller.dart

packages/google_maps_flutter/pubspec.yaml

ditman

Hello there! Thank you very much for the PR!

We're about to merge a change to google_maps_flutter, so it follows the "federated implementation" style. This will allow new platforms (like web) to be supported.

Once that change lands, some of the code that you've created/touched should live in the google_maps_flutter_platform_interface package, and not here.

Check out this PR on how we migrated the whole plugin to the new architecture. It should give you clues on how to modify your code to conform to it.

Feel free to reach out to me if you need any assistance with this!

packages/google_maps_flutter/lib/src/controller.dart

ditman · 2020-04-24T19:35:56Z

The refactor has landed.

googlebot · 2020-04-28T10:24:33Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

packages/google_maps_flutter/google_maps_flutter/pubspec.yaml

klaes-ashford · 2020-04-28T10:57:57Z

@googlebot I fixed it.

googlebot · 2020-04-28T10:58:07Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

klaes-ashford · 2020-04-28T20:35:18Z

@ditman @cyanglaz , I am integrating Federated plugin implementations changes to my open PR , and is facing some issues. Help resolving this is much appreciated.

I've added an api to the google_maps_flutter_platform_interface and bumped the version number according to sem versioning. But since this is not hosted anywhere I am getting this issue.
google_maps_flutter depends on google_maps_flutter_platform_interface ^1.0.2 which doesn't match any versions, version solving failed.

To solve this issue, I have to use relative path in my local project, rather than hosted one. But doing so will make my changes unpublishable in github tests.
Don't depend on "google_maps_flutter_platform_interface" from the path source. Use the hosted source instead.

and if I stick with the google_maps_flutter_platform_interface ^1.0.1, the new changes are not available for the CI and tests would fail.
error - The method 'getPoint' isn't defined for the type 'GoogleMapsFlutterPlatform' at lib/src/controller.dart:259:39 - (undefined_method)

Now, the tests are failing due to this. How should I solve this problem ?

ditman · 2020-04-28T20:59:35Z

@aswinkviswanath you're correct. In order for you to push this change, you'll need to first get the changes in the platform_interface package reviewed and published, then the changes in the core plugin. This requires two separate PRs.

It is normal to have a version of the core plugin that depends on the platform_interface via path, that is unpublishable until the platform_interface lands.

Let me know if you have more questions, and thanks for adopting the new style so quickly!

Make this depend on the version "from path" to check if all tests pass. This should make everything green except the "publishable" check.

ditman · 2020-05-04T19:14:30Z

I've introduced a change to this PR that should be reverted once the platform interface changes land, and are published to pub.dev. This is to make sure that all tests pass (except the "publishable" one, of course)

ditman · 2020-05-04T19:59:01Z

(It seems there's ~38 tests broken in Android, can you take a look at those @aswinkviswanath?)

+---------+------------------------+--------------------------------+
| OUTCOME |    TEST_AXIS_VALUE     |          TEST_DETAILS          |
+---------+------------------------+--------------------------------+
| Failed  | flame-29-en-portrait   | 38 test cases failed, 8 passed |
| Failed  | walleye-26-en-portrait | 38 test cases failed, 8 passed |
+---------+------------------------+--------------------------------+

Some traces (don't look very useful):

05-04 12:36:16.807: I/TestRunner(10192): started: testTraffic(io.flutter.plugins.googlemaps.EmbeddingV1ActivityTest)
05-04 12:36:16.808: I/MonitoringInstr(10192): Activities that are still in CREATED to STOPPED: 0
05-04 12:36:16.809: E/TestRunner(10192): failed: testTraffic(io.flutter.plugins.googlemaps.EmbeddingV1ActivityTest)
05-04 12:36:16.809: E/TestRunner(10192): ----- begin exception -----
05-04 12:36:16.809: E/TestRunner(10192): java.lang.Exception: failed
05-04 12:36:16.809: E/TestRunner(10192): 	at dev.flutter.plugins.e2e.FlutterRunner.run(FlutterRunner.java:62)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.Suite.runChild(Suite.java:128)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.Suite.runChild(Suite.java:27)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
05-04 12:36:16.809: E/TestRunner(10192): 	at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
05-04 12:36:16.809: E/TestRunner(10192): 	at androidx.test.internal.runner.TestExecutor.execute(TestExecutor.java:56)
05-04 12:36:16.809: E/TestRunner(10192): 	at androidx.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:392)
05-04 12:36:16.809: E/TestRunner(10192): 	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2074)
05-04 12:36:16.809: E/TestRunner(10192): ----- end exception -----
05-04 12:36:16.810: I/TestRunner(10192): finished: testTraffic(io.flutter.plugins.googlemaps.EmbeddingV1ActivityTest)

PS: Does your master branch contain all the changes in aswinkviswanath:feature/GetPoint?

klaes-ashford · 2020-05-04T20:34:59Z

@ditman flutter driver --target test_driver/google_maps_e2e.dart for android would fail due to MissingPluginException(No implementation found for method projection#getPointsForMeters on channel as getPointsForMeters is not implemented for android as the android google maps sdk itself doesn't expose the same. I was unable to see the getPointsForMeters in android projection api's documentation. Correct me if I am wrong, I assume this is the failure with integration tests. what should be done in this case ?

We could use the logic used in the driver test to verify getPointsForMeters to make an implementation for andriod, but I am not sure how accurate that is / if its the way to go.

and master branch contains all the changes in aswinkviswanath:feature/GetPoint.

ditman · 2020-05-04T22:02:10Z

@aswinkviswanath this is the list of failing tests in Android:

testRotateGesturesEnabled
testZoomGesturesEnabled
testSetMapStyle valid Json String
testSetMapStyle invalid Json String
testGetVisibleRegion
testScreenCoordinate
testInitialCenterLocationAtCenter
testTiltGesturesEnabled
testGetPointsForMeters
testTraffic
testGetLatLng
testSetMapStyle null string
testZoomControlsEnabled
testMapToolbarToggle
updateMinMaxZoomLevels
testCompassToggle
testScrollGesturesEnabled
testGetZoomLevel
testBuildings
testRotateGesturesEnabled
testZoomGesturesEnabled
testSetMapStyle valid Json String
testSetMapStyle invalid Json String
testGetVisibleRegion
testScreenCoordinate
testInitialCenterLocationAtCenter
testTiltGesturesEnabled
testGetPointsForMeters
testTraffic
testGetLatLng
testSetMapStyle null string
testZoomControlsEnabled
testMapToolbarToggle
updateMinMaxZoomLevels
testCompassToggle
testScrollGesturesEnabled
testGetZoomLevel
testBuildings

I'd understand testGetPointsForMeters fails, but I don't understand why the others do.

BTW, can you make testGetPointsForMeters test to be skipped when not run on iOS?

ditman · 2020-05-04T22:15:07Z

See an example of a skipped test.

klaes-ashford · 2020-05-05T06:04:46Z

@ditman skipped testGetPointsForMeters in non iOS platforms, and the integration tests seems to pass now.

ditman · 2020-05-05T17:13:17Z

@ditman skipped testGetPointsForMeters in non iOS platforms, and the integration tests seems to pass now.

Yes, this is great, thanks for taking care of this @aswinkviswanath!!

cyanglaz · 2020-05-15T17:00:41Z

Can we make this API available for android as well?

klaes-ashford added 3 commits February 6, 2020 01:07

added pointForMeters implemetention for iOS.

76a7779

added integration test for pointForMeters

27ebbcc

updated changelog and version.

e45832f

klaes-ashford requested a review from cyanglaz as a code owner February 5, 2020 21:18

googlebot added the cla: yes label Feb 5, 2020

klaes-ashford added 2 commits February 6, 2020 02:58

resolved conflicts and updated changelog and version.

f0be831

applied flutter_plugin_tools formatter patch to fix formatting issues.

f1264b4

klaes-ashford requested a review from iskakaushik February 7, 2020 02:51

cyanglaz suggested changes Feb 24, 2020

View reviewed changes

packages/google_maps_flutter/example/test_driver/google_maps_e2e.dart Outdated Show resolved Hide resolved

packages/google_maps_flutter/lib/src/controller.dart Outdated Show resolved Hide resolved

packages/google_maps_flutter/pubspec.yaml Outdated Show resolved Hide resolved

ditman suggested changes Apr 21, 2020

View reviewed changes

packages/google_maps_flutter/lib/src/controller.dart Outdated Show resolved Hide resolved

Aswin.K.V added 7 commits April 28, 2020 13:15

resolved conflicts from upstream master

89c2f51

moved getDistance to platform_interface

d7898d5

fixed interface and implementation for getDistance.

a1ef77c

added get distance to controller and tests.

4590704

fixed syntax issue in getDistance in controller

9fc32e5

formatted using flutter_plugin_tools

f5babcd

bumped google_maps_flutter_platform_interface to 1.0.2

09694ce

googlebot added cla: no and removed cla: yes labels Apr 28, 2020

klaes-ashford commented Apr 28, 2020

View reviewed changes

packages/google_maps_flutter/google_maps_flutter/pubspec.yaml Outdated Show resolved Hide resolved

googlebot added cla: yes and removed cla: no labels Apr 28, 2020

iskakaushik removed their request for review April 28, 2020 16:38

Aswin.K.V added 2 commits April 29, 2020 00:48

updated getDistance test to not use hardcoded pointForMeters

4aaef50

updated google_maps_flutter_platform_interface to use relative path.

2b80e0b

Aswin.K.V added 2 commits April 29, 2020 01:12

renamed getDistance to getPoint.

d56f01d

remove relative path for google_maps_flutter_platform_interface

56a4363

added getpoint to support projection method pointForMeters in iOS.

f1af845

klaes-ashford mentioned this pull request Apr 29, 2020

[google_maps_flutter_platform_interface] added getpoint to support projection method pointForMeters in iOS. #2719

Closed

13 tasks

Aswin.K.V and others added 6 commits May 2, 2020 10:19

method renamed to getPointsForMeters to closely follow native API

4ec34a1

formatted the dart files.

0a59700

resolved conflicts from getPoint branch.

0560b5a

method renamed to getPointsForMeters to closely follow native API

8750952

formatted the dart files.

8b611e8

Update pubspec.yaml

bad59e1

Make this depend on the version "from path" to check if all tests pass. This should make everything green except the "publishable" check.

Aswin.K.V added 2 commits May 5, 2020 09:47

Merge branch 'master' of https://github.com/flutter/plugins

1f9cdea

skip testGetPointsForMeters if not iOS implemented.

b46e906

Aswin.K.V added 4 commits May 15, 2020 10:17

conflict resolved.

aa33f19

bumped minor version.

1f23ead

resolved conflicts

347f172

resolved conflicts

500b775

klaes-ashford requested review from cyanglaz and ditman May 15, 2020 07:01

klaes-ashford closed this Aug 2, 2020

[google_maps_flutter] Add pointForMeters to plugin in iOS #2503

[google_maps_flutter] Add pointForMeters to plugin in iOS #2503

Uh oh!

Conversation

klaes-ashford commented Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Checklist

Breaking Change

Uh oh!

klaes-ashford commented Feb 7, 2020

Uh oh!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ditman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ditman commented Apr 24, 2020

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

Uh oh!

klaes-ashford commented Apr 28, 2020

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

klaes-ashford commented Apr 28, 2020

Uh oh!

ditman commented Apr 28, 2020

Uh oh!

ditman commented May 4, 2020

Uh oh!

ditman commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

klaes-ashford commented May 4, 2020

Uh oh!

ditman commented May 4, 2020

Uh oh!

ditman commented May 4, 2020

Uh oh!

klaes-ashford commented May 5, 2020

Uh oh!

ditman commented May 5, 2020

Uh oh!

cyanglaz commented May 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

klaes-ashford commented Feb 5, 2020 •

edited

Loading

ditman commented May 4, 2020 •

edited

Loading