search

stevage / map-gl-utils

A utility library that makes Mapbox GL JS or Maplibre GL a bit more convenient to work with.
https://npmjs.com/package/map-gl-utils
210 stars 24 forks source link

Hover popup mousemove #37

Open mccahan opened 2 years ago

mccahan commented 2 years ago

For your consideration, I needed to add in an additional argument to hoverPopup to allow me to change the mouse event for showing the popup to mousemove. When working with shapefiles that have overlapping area features or that otherwise don't have a gap between features, Mapbox only fires mouseenter when the mouse enters into an object of that layer, but not if the mouse moves from feature to feature in the same layer without ever leaving a feature completely. I wanted to be able to use your handy function, just have it trigger on mousemove instead in specific cases (the default works great for single-point features) to have the popup update content when moving between adjacent features without a gap.

It also can make for a nice user experience to have the tooltip following the mouse.

Demo

Using mouseenter, you can see it only updating when I move through an area where there are no features for that layer:

https://user-images.githubusercontent.com/2308923/171329556-1477c4ad-8df8-4743-bac2-f5de22d5bdb5.mp4

Setting the mode to mousemove instead:

https://user-images.githubusercontent.com/2308923/171329736-2c9ce8a7-b799-4bc6-8709-b1665a4406f4.mp4

Proposing dropping it at the end of the options list and defaulting it to mouseenter to maintain backwards-compatibility, but of course up to you if this is a welcome addition to your library.

Testing

I blind-wrote a test for it as well based on some of the other tests, but I couldn't get the tests to run using any combination of things I tried (Jest throws a SyntaxError: Cannot use import statement outside a module on the import statement in noflow/index.test.js even on a fresh download yarn install && yarn pretest && yarn test and other combinations of things on Node v17.8.0). Happy to revise if I'm running the tests wrong.

Codepens

stevage commented 2 years ago

Thanks, this looks awesome!

blind-wrote a test for it as well based on some of the other tests, but I couldn't get the tests to run using any combination of things I tried (Jest throws a SyntaxError: Cannot use import statement outside a module on the import statement in noflow/index.test.js even on a fresh download yarn install && yarn pretest && yarn test and other combinations of things on Node v17.8.0). Happy to revise if I'm running the tests wrong.

It's been a while since I looked at this, but yeah, I had similar problems to you.

I've just done some fiddling around, looking at the solutions here and think I have solved it. I've pushed a commit to master, can you check that works for you? (run yarn install again...)

mccahan commented 2 years ago

No dice for me, new error on testing (from yarn install on a fresh copy of your main branch):

yarn run v1.22.17
$ node_modules/rollup/dist/bin/rollup -c rollup.config.js

src/index.js → umd/index.js, umd/index.min.js...
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
[!] (plugin babel) Error: Rollup requires that your Babel configuration keeps ES6 module syntax intact. Unfortunately it looks like your configuration specifies a module transformer to replace ES6 modules with another module format. To continue you have to disable it.

Most commonly it's a CommonJS transform added by @babel/preset-env - in such case you should disable it by adding `modules: false` option to that preset (described in more detail here - https://github.com/rollup/plugins/tree/master/packages/babel#modules ).
src/index.js
Error: Rollup requires that your Babel configuration keeps ES6 module syntax intact. Unfortunately it looks like your configuration specifies a module transformer to replace ES6 modules with another module format. To continue you have to disable it.

Most commonly it's a CommonJS transform added by @babel/preset-env - in such case you should disable it by adding `modules: false` option to that preset (described in more detail here - https://github.com/rollup/plugins/tree/master/packages/babel#modules ).
    at error (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/rollup/dist/shared/rollup.js:5305:30)
    at throwPluginError (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/rollup/dist/shared/rollup.js:18033:12)
    at Object.error (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/rollup/dist/shared/rollup.js:18659:20)
    at Object.error (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/rollup/dist/shared/rollup.js:18202:38)
    at preflightCheck (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/@rollup/plugin-babel/dist/index.js:196:9)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at /Users/mccahan/Downloads/map-gl-utils-master/node_modules/@rollup/plugin-babel/dist/index.js:440:13
    at transformCode (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/@rollup/plugin-babel/dist/index.js:245:24)
    at ModuleLoader.addModuleSource (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/rollup/dist/shared/rollup.js:18396:30)
    at ModuleLoader.fetchModule (/Users/mccahan/Downloads/map-gl-utils-master/node_modules/rollup/dist/shared/rollup.js:18452:9)

error Command failed with exit code 1.

In my sleep deprivation I also managed to wreck the commit history on this PR so if you do end up merging it if you wouldn't mind making sure to do a squash on it that would be great 😂

stevage commented 2 years ago

Interesting. Man it is really hard getting all this stuff to play nicely together.

Also, to be honest, I don't think I have the headspace to sort out someone else's git history right now. If you want to make a new PR I'll try to take a look.