search

OwnWeb / flutter_map_tappable_polyline

A tappable Polyline plugin for flutter_map
https://pub.dev/packages/flutter_map_tappable_polyline
MIT License
54 stars 44 forks source link

[3.x] Overlapping line support #28

Closed FaFre closed 3 years ago

FaFre commented 3 years ago

Added support for overlapping lines. This will change the signature of the onTap callback.

tuarrep commented 3 years ago

Thanks, @FaFre, this PR should be a great addition 👍 The code looks good to me at first read.

I'm concerned about breaking BC. IMO, we have too few changes to make a major version.

Let me think to a way to be backward compatible before merging. Ideas are welcome.

FaFre commented 3 years ago

Sure! I'm not entirely certain, but since you didn't defined a clear Signature for onTap its maybe possible to do a type matching, if the argument is an List or single element. In that way it would be backward compatible.

tuarrep commented 3 years ago

I'll dig into that way this evening unless you did

tuarrep commented 3 years ago

I've dug this point.

We cannot infer type as suggested because of typing restriction here

https://github.com/FaFre/flutter_map_tappable_polyline/blob/31b9d4a6cf6a9cdee9c2fcd5bbd58e6c05b796a5/lib/flutter_map_tappable_polyline.dart#L33

More over, reflection options in Dart are quite hacky-ish.

I'm now in favour of a major.

HugoHeneault commented 3 years ago

It's a bit late as it has been merged, but I'm unsure about this solution which add complexity for edge-cases (imo).

Why not adding and onOverlapingTap callback which is triggered when set AND when when lines overlaps?