-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
More scattergl symbols #1482
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
More scattergl symbols #1482
Conversation
Thanks a lot! We should make these on-par with the SVG symbol list or at the very least we shouldn't add |
... and by the way please we don't need those |
@etpinard Sounds good, I reverted the devtools folder back to master. I attempted to find all the unicode symbols that I could to match the SVG symbol list, but couldn't find all of them, so this is the best I could do. I'm wondering, would it make sense to render the symbols on the |
And, I actually counted wrong, there's 35 more symbols so a total of 43 |
src/constants/gl_markers.js
Outdated
'square-x': '⊠', | ||
'cross-thin': '+', | ||
'asterisk': '✱' | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@destradafilm try running npm run lint-fix
That's ok. We don't have to have perfect parity between WebGL and SVG symbol. Like I said in #1482 (comment), we should just make sure that we don't introduce WebGL-only markers - which I think you did, but we should double check possible by adding a jasmine test. |
src/constants/gl_markers.js
Outdated
cross: '+', | ||
x: '❌' | ||
}; | ||
cross: '➕', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you changed the cross
symbol which is breaking the gl3d_marker_arrays
and the gl3d_projection_traces
mocks.
src/constants/gl_markers.js
Outdated
@@ -16,7 +16,7 @@ module.exports = { | |||
'square-open': '□', | |||
diamond: '◆', | |||
'diamond-open': '◇', | |||
cross: '➕', | |||
cross: '+', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed cross back to original symbol, so that means I removed 'cross-thin'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Reverting back to the current symbol is the way to go here.
That said, we could (and should I'd say) make cross
thicker and add cross-thin
in v2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to the #420 list of planned breaking changes.
Ok @destradafilm before merging we should make the new WebGL symbols have image test support. Can you try adding an If you're up for a challenge, you can also try generating the baseline images using our image test docker container. |
@destradafilm Are you ok with taking care of #1482 (comment) or do you me to do it? This would make a nice addition to next week's |
@etpinard I've been too busy in the past few days to work on it. If you're willing to do it, that would be great! If not, it will probably have to be the end of next week before I can get to it. |
@destradafilm no problem. I'll take care of it (should only be a 5-minute job)! Thanks again for the PR 🍻 |
Superseded by #1550 |
Added 49 more symbols to get as close to the 'scatter' plot symbol list to include :
previous list:

updated list:
