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

make feature tap detection work for features with a null id on ios (same as android) #357

Merged

Conversation

lunaticcoding
Copy link
Contributor

Feature Id nullable on android but not on iOS.

On android the feature id can be null which works fine in our app but on iOS clicking on points of interest the reaction is the same as clicking elsewhere on the map.

In the android code
Screenshot 2023-12-12 at 13 42 00

null is a valid value. I adapted the iOS code to allow for the same behaviour.

@m0nac0
Copy link
Collaborator

m0nac0 commented Dec 12, 2023

Sounds reasonable, thanks for contributing.

Do you happen to have tested if null ids also work on web?

@lunaticcoding
Copy link
Contributor Author

you're welcome :)

I just tried taking a look at web:

@JS()
@anonymous
class FeatureJsImpl {
  external dynamic get id;
  external set id(dynamic id);
  external String get type;
  external GeometryJsImpl get geometry;
  external dynamic get properties;
  external String get source;
  external factory FeatureJsImpl({
    dynamic id,
    String? type,
    GeometryJsImpl geometry,
    dynamic properties,
    String? source,
  });
}

As far as I can tell web seems to already support/allow null. I however have little experience with flutter web plugins.

@lunaticcoding
Copy link
Contributor Author

@m0nac0 is it possible to get this merged soon? :) We need it for a client project.

@m0nac0 m0nac0 changed the title make id nullable on ios (same as android) make feature tap detection work for features with a null id on ios (same as android) Dec 15, 2023
Copy link
Collaborator

@m0nac0 m0nac0 left a comment

Choose a reason for hiding this comment

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

There seem to be a few other places in the iOS code (e.g. onFeatureDrag) that seem to expect a non-null id.
But since it's already possible to add features with a null id and this fixes at least one issue with null-id-features, I'm approving it.

But please be cautioned that some other things may break if you set the id to null (e.g. I could imagine feature dragging on iOS not working correctly, but I haven't tested it). So adding features with id=null should be considered not to be officially supported.

@m0nac0
Copy link
Collaborator

m0nac0 commented Dec 15, 2023

Thanks again for contributing!

@m0nac0 m0nac0 merged commit 1fd4001 into maplibre:main Dec 15, 2023
6 checks passed
@josxha josxha added this to the v0.19.0 milestone May 20, 2024
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