Skip to content

Conversation

@Ph0tonic
Copy link
Contributor

@Ph0tonic Ph0tonic commented May 4, 2020

This PR is an implementation proposal for issue #966

What I choose is that clickListener should now return a boolean with the following meaning :

  • True if event should be consumed
  • False otherwise

When an event is consumed then no other local listener to the manager will be call as discussed.

@Ph0tonic
Copy link
Contributor Author

Ph0tonic commented May 5, 2020

@langsmith and @LukasPaczos I tried to fix the build on circleci and it asking me to run make generate-annotation-code locally. Unfortunately it fails with the following error message :

internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module './../gl-js/src/style-spec/reference/v8'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/mnt/d/Projects/mapbox-plugins-android/plugin-annotation/scripts/code-gen.js:6:14)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
Makefile:37: recipe for target 'generate-annotation-code' failed
make: *** [generate-annotation-code] Error 1

What do I need to do to make it work ?
Thanks in advance

@LukasPaczos
Copy link
Contributor

Changes look good to me, thanks for your contribution @Ph0tonic!

@langsmith could you help to regenerate the code and make a final review?

@LukasPaczos LukasPaczos requested a review from langsmith May 5, 2020 08:07
@Ph0tonic Ph0tonic closed this May 5, 2020
@Ph0tonic Ph0tonic reopened this May 5, 2020
@Ph0tonic
Copy link
Contributor Author

Ph0tonic commented May 5, 2020

@langsmith Thanks to you the build succeed. Just by curiosity, is there a reason to force to have all those imports ?

@Ph0tonic Ph0tonic changed the title Consumable event with ClickListener Consumable event with ClickListener (#966) May 5, 2020
@Ph0tonic Ph0tonic changed the title Consumable event with ClickListener (#966) Consumable event with ClickListener May 5, 2020
@langsmith
Copy link
Contributor

is there a reason to force to have all those imports ?

To make sure that our plugin is up-to-date with the Mapbox style spec

The current CircleCI crash is related to credentials and that's happening because you're a non-Mapboxer 😕 I'll just quickly spin up an equivalent pr with my account.

@Ph0tonic
Copy link
Contributor Author

Ph0tonic commented May 5, 2020

To make sure that our plugin is up-to-date with the Mapbox style spec

Cool, thanks

The current CircleCI crash is related to credentials and that's happening because you're a non-Mapboxer 😕 I'll just quickly spin up an equivalent pr with my account.

Ok, wouldn't it work if you relaunch the release build manually ?

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