feat: add onClick event for map

[jabranr]

Hi,

This PR adds onClick event support for the map. This addresses the #49.

As part of this PR I have:

• add an example
• updated API reference
• wrote unit test coverage

Unit tests
I have tried to put a basic unit test but it just won't work so I would appreciate the help/feedback with this. Here is the example I tried with:

test('onClick handler is called when map is clicked', () => {
  createMapSpy.mockReset();
  const onClick = jest.fn();
  rerender(
    <GoogleMap id={'mymap'} center={center} zoom={14} onClick={onClick} />
  );

  screen.getByTestId('map').click();
  expect(onClick).toHaveBeenCalledTimes(1);
});

[usefulthink]

First of all, thanks so much for taking the time and contributing to this project. Also congratulations, you are the very first outside contributor here 🎉

Adding the map-events was actually something that I had on my todo list for the next week, but I would be more than happy to hand this task over to you or collaborate in this PR to get it done.

I left a number of comments for you to consider with the overall vision for the event-support in mind. If you need any help, feel free to let me know, otherwise I'd try to pick it up from here and add to this PR.

[usefulthink]

I think the events we add here should use the same signatures as the events defined in the google maps API, see here: https://developers.google.com/maps/documentation/javascript/reference/map#Map-Events

At least for now I don't think we need anything on top of providing a props-version for the events coming from the maps-API.

So in this case that would be

Suggested change

	onClick?: (event: google.maps.Map) => void;
	onClick?: (event: google.maps.MapMouseEvent | google.maps.MapIconEvent) => void;

[usefulthink]

see my note above:

Suggested change

	onClick?: (event: google.maps.Map) => void;
	onClick?: (event: google.maps.MapMouseEvent | google.maps.MapIconEvent) => void;

[usefulthink]

I don't quite understand why this is added here. As far as I understand it, ComponentPropsWithRef is supposed to be used when a component wraps a regular HTML-element, but that isn't what is happening here.

If anything, we're wrapping the google.maps.Map instance (using forwardRef to expose the map-object is something I wanted to add).

[usefulthink]

What is the reasoning for waiting for the idle-event before adding the click-event?

[usefulthink]

This is missing some code to remove the event-listener on unmount. For this, we could store the returned google.maps.MapsEventListener object and call it's remove() method.

Since there's going to be a lot of events, I think this should be extracted into a separate internal hook (for example useMapEventHandlers() that takes care of adding and removing event-handlers based on the props. Something similar has been done in src/components/advanced-marker.tsx.

[usefulthink]

There's going to be a lot more events added and I would like to have a single example to demonstrate them. With that in mind, could you rename the example to map-events or similar?

[usefulthink]

As for the unit-tests, that's a bit complicated since we don't actually have a real map-instance to work with (everything google-maps related comes from the @googlemaps/jest-mocks library).

What we need to do here is to mock emitting the events, which can get a bit complicated.

I'd try the following:

• In the test-environment, google.maps.event.addListener is actually a jest.fn() spy.
• After the map is initialized, we can find the callbacks passed in by the Map component using something like

  const addListenerMock = google.maps.event.addListener as jest.Mock;
  const [name, callback] = addListenerMock.calls.find(args => args[0] === 'click');`

• after that, the callback can be called and verified that the function passed to the onClick prop is actually called

Something similar can be done to validate that event-listeners are removed on unmount and re-render.

Unfortunately we need to be very specific here about which parts of the maps-api are used since the @googlemaps/jest-mocks are fully non-functional. So using map.addListener instead of google.maps.event.addListener would break the test even though both are equivalent when it comes to the real thing. But that is something we have to live with for now.

[jabranr]

@usefulthink Thank you Martin for the feedback. I will pick it up and will revert.

[usefulthink]

I'm very sorry for this, but I had to move on with this topic since it's one of the biggest bits of functionality currently missing.

The implementation for full event-support is almost ready in this PR: #64
I still have to add some documentation for this and add an example. Do you want to pitch in with that?

[usefulthink]

Still missing tests and documentation, but the main functionality for events is now merged and available in v0.3.
Again, apologies for rushing this but I had to get this done asap.

[jabranr]

No worries! I have been quite busy recently so it was hard to take time out. Appreciate the update as you went along. I will be glad to contribute to any other parts of this repo!