Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Returns layer Id on Feature Tap #475

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

LouisRaverdy
Copy link

Hello MapLibre Team !

I needed to retrieve the layer Id of the feature tapped, so I've implemented it.
This is much cleaner than using the given Id to have a unique identifier and the layer id.

Best regards,

Louis

Copy link

@Zverik Zverik left a comment

Choose a reason for hiding this comment

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

generally ok, some code questions

final snackBar = SnackBar(
content: Text(
'Tapped feature with id $featureId',
'Tapped feature with id $featureId on payer $layerId',
Copy link

Choose a reason for hiding this comment

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

*layer

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'Tapped feature with id $featureId on payer $layerId',
'Tapped feature with id $featureId on layer $layerId',

@@ -56,9 +56,9 @@ class MapLibreMapController: NSObject, FlutterPlatformView, MLNMapViewDelegate,
target: self,
action: #selector(handleMapTap(sender:))
)
for recognizer in mapView.gestureRecognizers! where recognizer is UITapGestureRecognizer {
/*for recognizer in mapView.gestureRecognizers! where recognizer is UITapGestureRecognizer {
Copy link

Choose a reason for hiding this comment

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

Why did you comment it out?

Copy link
Author

Choose a reason for hiding this comment

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

My bad, this was to enhanced the speed of the callback, with this commented the interaction callback switch from 1s to 0.1s. I was unable to remove all the slow gestureRecognizers

Copy link

Choose a reason for hiding this comment

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

Workaround is still there — did you find another reason to keep it?

Copy link
Author

Choose a reason for hiding this comment

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

No, but I haven't been able to find a good solution for this important delay. Perhaps you have an idea?

@@ -8,7 +8,7 @@ typedef OnMapClickCallback = void Function(
Point<double> point, LatLng coordinates);

typedef OnFeatureInteractionCallback = void Function(
dynamic id, Point<double> point, LatLng coordinates);
dynamic id, Point<double> point, LatLng coordinates, String layerId);
Copy link

Choose a reason for hiding this comment

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

My issue with this is that it changes the callback function signature. Fine since the library is not stable yet. But maybe it's type to make a InteractionEvent class to package all the attributes inside?

Copy link
Author

Choose a reason for hiding this comment

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

Why not, but I don't think it's necessary, given that functionality is vital.

@josxha josxha added this to the v0.21.0 milestone Oct 3, 2024
Copy link
Author

@LouisRaverdy LouisRaverdy left a comment

Choose a reason for hiding this comment

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

Remove workaround

@josxha
Copy link
Collaborator

josxha commented Nov 11, 2024

Hi @LouisRaverdy, I agree this is a much needed feature. Through, there are still unresolved conversations and it introduces breaking API changes. I'm fine if we skip 0.20.1 and directly go for a breaking 0.21.0 release but please ensure that all open conversations are solved, then I can review and merge your pr.

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

Successfully merging this pull request may close these issues.

3 participants