Skip to content

Conversation

@barbeau
Copy link
Collaborator

@barbeau barbeau commented Jun 22, 2020

Follow up to implementing the feature in #752.

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
  • Will this cause breaking changes to existing Java or Kotlin integrations? If so, ensure the commit has a BREAKING CHANGE footer so when this change is integrated a major version update is triggered. See: https://www.conventionalcommits.org/en/v1.0.0/

@barbeau barbeau requested a review from arriolac June 22, 2020 13:29
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 22, 2020
@codecov
Copy link

codecov bot commented Jun 22, 2020

Codecov Report

Merging #754 (e4d4592) into main (7a9c747) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #754   +/-   ##
=======================================
  Coverage   39.20%   39.20%           
=======================================
  Files          71       71           
  Lines        4076     4076           
  Branches      609      609           
=======================================
  Hits         1598     1598           
  Misses       2375     2375           
  Partials      103      103           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a9c747...e4d4592. Read the comment docs.

Toast.makeText(ClusteringDemoActivity.this,
"Info window clicked.",
Toast.LENGTH_SHORT).show());
mClusterManager.getMarkerCollection().setOnInfoWindowLongClickListener(marker ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be preferred over setting listeners through mClusterManager.setOnClusterItemInfoWindowLongClickListener/mClusterManager.setOnClusterInfoWindowLongClickListener?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, good question. To be honest I was following the existing pattern for setOnInfoWindowClickListener() - let me take a closer look.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@barbeau , may I apply this change? I think setting the listener directly on the Cluster makes more sense - I can update this file to reflect that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To be honest it's been a while since I've looked at this, and I recall it not being straightforward but I don't recall the details. I'll trust your judgment on what you think is the best path forward. 😁

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have added the listener directly to the Cluster. @wangela , if you have some time a review here would be appreciated.

@arriolac arriolac changed the base branch from master to main September 14, 2021 21:12
@kikoso kikoso force-pushed the sean/demo-long-click branch from e4d4592 to 5c2d193 Compare January 5, 2023 14:38
@kikoso kikoso force-pushed the sean/demo-long-click branch from 3a72c95 to aa98df0 Compare January 13, 2023 18:55
@wangela wangela merged commit ab990d5 into main Jan 13, 2023
@wangela wangela deleted the sean/demo-long-click branch January 13, 2023 19:02
@googlemaps-bot
Copy link
Contributor

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants