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

feat: introduce feature drag and pointer events handling #162

Open
wants to merge 38 commits into
base: next
Choose a base branch
from

Conversation

gabbopalma
Copy link
Contributor

@gabbopalma gabbopalma commented Dec 4, 2024

Closes #151 issue.

Important

This PR doesn't introduce the implementation of drag gestures on Web or iOS platforms.

@gabbopalma gabbopalma marked this pull request as draft December 4, 2024 16:32
@rmmlopes
Copy link

rmmlopes commented Jan 5, 2025

Hi, I'm working on a project that needs to update polygons on the map. This PR would make that possible.

Any idea when it will be merged in?

@gabbopalma
Copy link
Contributor Author

Hi, I'm working on a project that needs to update polygons on the map. This PR would make that possible.

Any idea when it will be merged in?

Hi. I have currently stopped work because of the Christmas holidays. From January 7 I will resume work on this PR. I am close to the end of the work

@rmmlopes
Copy link

rmmlopes commented Jan 6, 2025

Hi, I'm working on a project that needs to update polygons on the map. This PR would make that possible.
Any idea when it will be merged in?

Hi. I have currently stopped work because of the Christmas holidays. From January 7 I will resume work on this PR. I am close to the end of the work

Awesome! Thanks!

@gabbopalma
Copy link
Contributor Author

gabbopalma commented Jan 10, 2025

I decided not to introduce the Web implementation in the current PR.
The equality problems encountered in switching from GeometryCollection to FeatureCollection have been solved.
Currently I can't solve ktlint quality control, I kindly ask @josxha if he can perform it for me.

Implementation of a new way of handling gestures and, consequently, iOS and Web support will follow in a future PR.
At the moment, if @josxha approves it, I will introduce the “Android only works” warning in the code.

@gabbopalma gabbopalma marked this pull request as ready for review January 10, 2025 19:40
@josxha
Copy link
Owner

josxha commented Jan 19, 2025

Hi @gabbopalma, I'm sorry for your long wait. I've currently a long going on and probably won't find the time until February for an in-depth review. Though, I did a quick scroll-though and your pr looks promising!
ktlint has a command to fix the formatting issues automatically but I can do it as you requested (can't remember the command at the moment).

@gabbopalma
Copy link
Contributor Author

gabbopalma commented Jan 29, 2025

I tested my code on a potential use case and I think a filter for interactive layers needs to be added.
Currently all features are detected, even those of style.json and this may be useless for most use cases for which MapEventFeatureDrag is used (like for moving and editing features).
We need to decide how and where to implement the interactive layers field @josxha

Also, I ran ktlintformat but I used version 0.4.17 and I think the version of ktlint is different from the pipeline used. Do we want to update it, or do I try to format it as the current version used?

@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2025

Codecov Report

Attention: Patch coverage is 57.25191% with 112 lines in your changes missing coverage. Please review.

Project coverage is 42.89%. Comparing base (abbab6f) to head (6455959).

@josxha
Copy link
Owner

josxha commented Jan 29, 2025

Not sure why codecov lost its configuration. 👀

We need to decide how and where to implement the interactive layers field

Could you elaborate a bit please? I'm currently not up-to-date. The queryRenderedFeatures has a filter, hasn't it? It the question how to make use of it when dragging a feature?

Also, I ran ktlintformat but I used version 0.4.17 and I think the version of ktlint is different from the pipeline used. Do we want to update it, or do I try to format it as the current version used?

It should use the following ruleset:

ktlintRuleset("io.nlopez.compose.rules:ktlint:0.4.+")

Maybe the dependencies were not loaded?

edit: not sure why it alters, I'll try when I find the time for a code review

@gabbopalma
Copy link
Contributor Author

We need to decide how and where to implement the interactive layers field

Could you elaborate a bit please? I'm currently not up-to-date. The queryRenderedFeatures has a filter, hasn't it? It the question how to make use of it when dragging a feature?

Yes, queryRenderedFeatures has a filter, but it is not currently used. I was thinking if we want the user to pass to MapLibreMap, or somewhere else, the list of layers that will then be queried by the MapEventFeatureDrag.
Currently, the fastest solution, but with less customization, would be to pass all the layers that are present in the LayerManager, but this wouldn't make the features present in the layers added with StyleLayer queryable.

Maybe the dependencies were not loaded?

I will try again. Maybe my Android Studio version has some conflicts with this. I was using it to run the gradle commands.

@josxha
Copy link
Owner

josxha commented Jan 29, 2025

I see. Yes, I agree it could be good to have fine control over what layers can get dragged and what can't. I've seen that the it is currently defined in MapOptions.gestures. How about moving this setting to Layer.draggable? Maybe even an enum that allows to specify the mode (on short press, on a long press, disabled). Would it work that way?

@gabbopalma
Copy link
Contributor Author

gabbopalma commented Jan 29, 2025

I see. Yes, I agree it could be good to have fine control over what layers can get dragged and what can't. I've seen that the it is currently defined in MapOptions.gestures. How about moving this setting to Layer.draggable? Maybe even an enum that allows to specify the mode (on short press, on a long press, disabled). Would it work that way?

Yes. I think Layer.draggable (or even in StyleLayer) may be the best way to handle this. I will implement it.

update: I finally got it done with ktlintFormat. Eureka!
Now the problems are related to codecov, but I don't know how I can help you. Can you check it out?

@josxha josxha added this to the v0.3.0 milestone Jan 30, 2025
@gabbopalma
Copy link
Contributor Author

gabbopalma commented Feb 1, 2025

I think I have finished my work here. I tried adding some tests for better coverage about Codecov.

I will wait for your review :)

@josxha josxha self-requested a review February 13, 2025 22:03
@josxha josxha changed the base branch from main to next February 13, 2025 22:15
Copy link
Owner

@josxha josxha left a comment

Choose a reason for hiding this comment

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

Hi @gabbopalma, sorry for your long wait. You pull request contains a lot of good stuff. Can you check my comments please?

Could you give me some directions for testing, please? I wasn't able to test the drag functionality in the example app. Is there an example I can use for testing?

I rebased the pr to a new next branch for the v0.3.0 release. WIth the release of Flutter 3.29.0 I'd like to sneak in another feature release.

Thanks for your work!

edit: also, don't worry about the integration test CIs. There is something odd going on #220.

lib/src/layer/layer_manager.dart Outdated Show resolved Hide resolved
lib/src/map_events.dart Outdated Show resolved Hide resolved
lib/src/platform/android/extensions.dart Outdated Show resolved Hide resolved
lib/src/platform/map_state_native.dart Outdated Show resolved Hide resolved
lib/src/map_controller.dart Outdated Show resolved Hide resolved
@josxha josxha modified the milestones: v0.2.1, v0.3.0 Feb 14, 2025
@gabbopalma
Copy link
Contributor Author

Hi @josxha. Thank you for your review :)
As requested, I fixed all the issues you had reported within the comments, and while I was at it I applied the following changes:

  • Refactor of the StyleController and implementation of its draggableLayers. I preferred a method rather than a list allocated within the interface.
    That way we have more control over the getter and setter of that list.
  • Add examples in app regarding interactive Layers and StyleLayers.
  • Added new “layerId” and “sourceId” fields within Layer and to all its subclasses.
    This allows the override of getLayerId and getSourceId, in case the developer wants to apply a specific id.

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

Successfully merging this pull request may close these issues.

[FEATURE] Provide a way to handle feature tap and drag events
4 participants