Conversation
... mostly to hide mapbox-gl's own no-css warning
that get injected into the DOM.
... to scattermapbox traces, along with fixing existing bugs
and cleaning up the convert logic (in 0.44.0, we no longer
have to generate a hacky hash stops array 🎉
- by setting up paint and layout props during the 1st addLayer call and by setting up the source during the 1st addSource call, as opposed to having an blank 'init' and a generic 'update' step.
|
Happy to manually test this tonight or tomorrow on stage! |
What do you mean by this? Just that we have tests that can't run on CI but can run locally, or are there things we have to run in a browser and eyeball? |
I added a command in |
- see #2343 (comment) for more details on this issue.
|
After some testing help from @jackparmer, tagging this thing as |
| expect(evt.clientX).toEqual(pointPos[0], 'event.clientX'); | ||
| expect(evt.clientY).toEqual(pointPos[1], 'event.clientY'); | ||
| expect(evt.clientX).toBeDefined('event.clientX'); | ||
| expect(evt.clientY).toBeDefined('event.clientY'); |
There was a problem hiding this comment.
why did these have to change?
There was a problem hiding this comment.
Oops. My mistake. I shouldn't have committed this. Good 👁️ Reverted in 0ef2ba6
| @@ -46,6 +46,29 @@ | |||
| "opacity": 0.5 | |||
There was a problem hiding this comment.
Looks (from the baseline image change) like this didn't actually work before, but now it does 🎉
Was that a known bug?
There was a problem hiding this comment.
Yes (after some initial confusion) -> #2346
src/traces/scattermapbox/hover.js
Outdated
|
|
||
| function wrapLon(lon) { | ||
| return Lib.wrap180(lon); | ||
| } |
There was a problem hiding this comment.
this wrapper seems unnecessary
|
|
||
| drawing.selectedPointStyle = function(s, trace) { | ||
| if(!s.size() || !trace.selectedpoints) return; | ||
| drawing.makeSelectedPointStyleFns = function(trace) { |
There was a problem hiding this comment.
Nice abstraction here 👌
|
@etpinard this looks great! We'll need to sort out fonts before we can use 💃 after the one last minor comment and tests pass. |
Ok. Referencing #1972 |
3 participants
resolves #1714 and makes #2029 obsolete.
It's about time we do this. Note that
mapbox-gl@0.44.0is still incompatible withnw@0.12that we use in our imagetest container (ref #2029 (comment)), meaning we'll have to manually test mapbox subplot image generation. But yeah after some discussing with @jackparmer, this is way better than sticking to a almost two year old version ofmapbox-gl.A lot of things have changed from
mapbox-glversion0.22.0to0.44.0. Our tests did catch many of those changes. But still, we'll need to manually test this thing thoroughly before merging.This PR also adds support for (un)selected marker size and color on top of marker opacity for scattermapbox traces #2346, fixes a hover bug and improves first-render performance.
Some TODOs: