Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[google_maps_flutter]add support for map tapping #985

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

RafaO
Copy link
Contributor

@RafaO RafaO commented Dec 13, 2018

Deliver the tap event in the map with the LatLng object where it happened.

Enables something like this:

      GoogleMap(
        onTap: (LatLng location) {
          showDialog(context: context, builder: (BuildContext context) {
            return AlertDialog(
              title: new Text("Map tapped!"),
              content: new Text("You tapped in " + location.latitude.toString() + ", " + location.longitude.toString()),
              actions: <Widget>[
                // usually buttons at the bottom of the dialog
                new FlatButton(
                  child: new Text("Close"),
                  onPressed: () {
                    Navigator.of(context).pop();
                  },
                ),
              ],
            );
          });
        },
      )

@RafaO RafaO force-pushed the map-tap-support branch 2 times, most recently from de5cedc to 9d896d8 Compare December 13, 2018 21:03
@nandymandy1
Copy link

I m not getting the idea of long tapped event on map to place marker on the tapped point listing or the Coordinates

@RafaO
Copy link
Contributor Author

RafaO commented Feb 11, 2019

Hey @nandymandy1 ! This PR is just delivering the tap event. It has nothing to do with long tapped events.

@nandymandy1
Copy link

how can i get your codes as I m in tight development schedule and how can i get access to your repository???

@RafaO
Copy link
Contributor Author

RafaO commented Feb 12, 2019

you can point to my fork like this in your pubspec.yaml:

  google_maps_flutter:
    git:
      url: git://github.com/RafaO/plugins.git
      ref: integration-branch
      path: packages/google_maps_flutter

But do it under your own risk! :)

@nandymandy1
Copy link

ontap listener ready?????

@nandymandy1
Copy link

can you show how to use it???

@RafaO
Copy link
Contributor Author

RafaO commented Feb 20, 2019

I updated the PR description including an example of usage @nandymandy1 hope it is useful

@cyanglaz cyanglaz changed the title add support for map tapping [google_maps_flutter]add support for map tapping Feb 23, 2019
@ghost
Copy link

ghost commented Mar 7, 2019

can u please update this and re-publish it ?

@RafaO RafaO force-pushed the map-tap-support branch 2 times, most recently from 9bcf88d to ae58d52 Compare March 7, 2019 20:12
@ebeem
Copy link

ebeem commented Mar 9, 2019

thanks for your contribution!!
I hope this PR gets merged soon, I believe getting location on tap/long-tap is a very essential feature

I am sorry that you're being asked this frequently, but I will appreciate it if you take some time to update your branch again as some APIs have changed.

also, I tried the snippet you posted, it looks like the listener is on the controller (not the google maps object itself) which seems an excellent decision in my point of view. here is a snippet that will add a marker and print tapped location, please consider updating your snippet so those who want to test your branch do not face any troubles

      mapController.onMapTapped.add((LatLng location) {
        print("tapped location ${location.latitude} ${location.longitude}");
        mapController.addMarker(new MarkerOptions(
          icon: BitmapDescriptor.defaultMarker,
          position: location,
        ));
      });

let me know once you update your fork so I update the snippet too, since the new API changes will break this snippet as well

@RafaO RafaO force-pushed the map-tap-support branch from ae58d52 to 82de64b Compare March 9, 2019 21:12
@RafaO
Copy link
Contributor Author

RafaO commented Mar 9, 2019

Hi @ebeem Thank you for your comment, I also hope this gets merged soon :)

I am not sure if I understood you completely.

I think this branch is already rebased to the newest changes on master. The controller is no longer used for listeners but the google map object itself. (It used to be the other way around) So I think the snippet is right. In fact, you can check the integration-branch of my fork which is still using the controller as you just described. Let me know if you still have a different mind :)

@ebeem
Copy link

ebeem commented Mar 9, 2019

@RafaO what branch are you actively updating?
it seems like the master is up-to-date, but the integration-branch is not
the master doesn't introduce the new listener onMapTap, while integration-branch does
but integration branch has some old APIs from old flutter google maps (23 days ago)

google_maps_flutter add support for map tapping 23 days ago

@RafaO
Copy link
Contributor Author

RafaO commented Mar 9, 2019

@ebeem the branch I actively update is map-tap-support the integration branch is what I use privately in my projects with support for map tapping and this other PR #981 But didn't update it yet.

@ebeem
Copy link

ebeem commented Mar 9, 2019

tested, working great
Thanks

@iskakaushik iskakaushik self-requested a review March 29, 2019 21:43
Copy link
Contributor

@iskakaushik iskakaushik left a comment

Choose a reason for hiding this comment

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

@RafaO , thanks for the PR. We have now added support for on-device tests for google maps plugin. Please take a look at this file for an example test case. I will be happy to help you through any issue you run into while adding tests. Please add some relevant tests to your PR :-)

one suggestion for the test case would be to:

  1. Create a map
  2. Tap the map and make sure the map gets tapped.

Here is an example of how you would do it if it were a button:

final LiveWidgetController widgetController = LiveWidgetController(WidgetsBinding.instance);

  test('tapButton', () async {
    bool isPressed = false;

    await pumpWidget(Directionality(
      textDirection: TextDirection.ltr,
      child: Material(
        child: RaisedButton(
          onPressed: () {
            isPressed = true;
          },
          child: const Text(
            "Hello, World!",
            key: ValueKey<String>("text"),
          ),
        ),
      ),
    ));

    await widgetController.tap(find.byKey(const ValueKey<String>("text")));

    expect(isPressed, true);
  });

Thanks again!

@iskakaushik
Copy link
Contributor

@RafaO , I can drive this PR to completion if you do not have the cycles/time to do it. Please allow changes to your branch. Follow the instructions at: https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork

@RafaO
Copy link
Contributor Author

RafaO commented Apr 3, 2019

Hi @iskakaushik thanks for your feedback and help. The checkmark for allowing changes from maintainers is already activated (was also this way), I hope you are able to perform changes and of course feel free to do so.

So far I have this:

  test('testMapTap', () async {
    LatLng place;
    bool pressed = false;

    final ValueKey<String> mapKey = const ValueKey<String>("map");

    await pumpWidget(Directionality(
        textDirection: TextDirection.ltr,
        child: GoogleMap(
          initialCameraPosition: _kInitialCameraPosition,
          onMapTapped: (LatLng latLng) {
            place = latLng;
            pressed = true;
          },
        ),
        key: mapKey));

    await widgetController.tap(find.byKey(mapKey));

    expect(pressed, true);
  });

But is not working, neither with the boolean or with the place being not null.

I am also not sure what is the best way of running these tests because the device stays stuck at the end of tests.

Thank you.

Copy link
Contributor

@iskakaushik iskakaushik left a comment

Choose a reason for hiding this comment

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

@RafaO , I looked into it. We do not support gestures on Platform Views yet. This is not a trivial problem because iOS does not let us synthesize gestures. As we don't have the infrastructure in place to effectively test this just yet. I filed an issue: flutter/flutter#30471

There are a couple of changes that I think will make this PR good to merge. Thanks again for this excellent contribution.

@RafaO RafaO force-pushed the map-tap-support branch from 82de64b to 085e88f Compare April 4, 2019 20:34
@RafaO
Copy link
Contributor Author

RafaO commented Apr 4, 2019

Hi @iskakaushik thanks for taking a look and creating the issue. I reworked with your suggestions the PR. Let me know what you think.

@iskakaushik
Copy link
Contributor

I think you misread my suggestion as onMapTap, I think just onTap is better api.

@RafaO RafaO force-pushed the map-tap-support branch from 085e88f to db2419f Compare April 4, 2019 21:39
Copy link
Contributor

@iskakaushik iskakaushik left a comment

Choose a reason for hiding this comment

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

image

@iskakaushik iskakaushik merged commit ce18897 into flutter:master Apr 4, 2019
@iskakaushik
Copy link
Contributor

iskakaushik commented Apr 4, 2019

Will shortly publish 0.5.2 which should have this. Thanks for this, @RafaO 🥇

@iskakaushik
Copy link
Contributor

#1447

@RafaO RafaO deleted the map-tap-support branch April 5, 2019 08:56
@kjeremy
Copy link

kjeremy commented Apr 5, 2019

Finally.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants