ibrierley / flutter_map_line_editor

A basic line/poly editor that works with flutter_map and dragmarkers.
MIT License
44 stars 25 forks source link

Add current point to callbackRefresh #42

Closed EricKrg closed 5 months ago

EricKrg commented 6 months ago

Hi @ibrierley ,

would it be possible to add the currently manipulated/edited point to the callbackRefresh Function? There are not a lot changes needed for that, but I stumble across the need for that functionality in a recent Project (see the video attached).

I made all the necessary changes in this PR and if you agree with the functionality I can also add a complete example.

I am also interested if you have an better idea to achieve this :)

Here the example, why I needed this functionality. The small PiP Map uses the point from the callbackRefresh function to serve as a kind of Map Lens. https://github.com/ibrierley/flutter_map_line_editor/assets/31648877/e0b30db3-9f37-4b91-b444-e44dd0f74055

ibrierley commented 6 months ago

Hiya, I'm probably not going to get chance to look at this properly until at least next week apologies. I don't think I see anything in principle I think is an issue, other than it's likely a breaking change ?

As an alternative, I wonder if you could use a closure for a function that's passed to the callbackRefresh that has the points enclosed in scope, but that may well be suboptimal.

Thanks for the PR!

EricKrg commented 6 months ago

I did not think of introducing breaking changes, but you are correct!

An alternative could be introducing another callback function besides callbackRefresh, to avoid breaking changes

Like this:

class PolyEditor {
//[...]
  final void Function()? callbackRefresh; // nothing changed
  final void Function({required LatLng updatePoint})? currentPointRefresh; // new callback

  // [...]
  void updateMarker(details, point) {
    if (_markerToUpdate != null) {
      points[_markerToUpdate!] = LatLng(point.latitude, point.longitude);
    }
    callbackRefresh?.call();
    currentPointRefresh?.call(
        updatePoint: LatLng(point.latitude, point.longitude));
  }
  // [...]
}

I'm probably not going to get chance to look at this properly until at least next week apologies.

don't worry its not urgent, just let me know if you like the idea, and have a look at it if you have time. I can prepare everything else

ibrierley commented 5 months ago

Yes, I don't really have a problem with the extra function param addition (I hate my original function name name :D). I don't think there's masses of people using the plugin anyway, and I'd rather let it be flexible for those who do use it. Shout also if you'd like to help maintain it, as I have less time atm. So feel free to amend!

EricKrg commented 5 months ago

Sounds great, thank you. I will add a complete example this week. In terms of maintaining it, I would definitely like to help out, but since I also work full-time I probably won't have a lot time to add new features and the like to it, but fixing dependencies and potential bugs is something I can do.

EricKrg commented 5 months ago

@ibrierley Apologies for the delay, but I have made some refinements to the merge request and added an example that demonstrates how the update callback can be utilized. Please feel free to merge it or suggest any improvements.

ibrierley commented 5 months ago

No problem, I'll try and get it tested before too long, thanks for the update.