-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add "bold" weight
, "italic" style
and "small-caps" variant
to fonts
#6956
Conversation
editType: 'calc', | ||
colorEditType: 'style', | ||
arrayOk: true, | ||
variantValues: ['normal', 'small-caps'], |
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.
Is there a reason for limiting the variants here?
Or, should we maybe just only support these two variants everywhere?
Not sure just wondering.
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.
Aha, this is because of WebGL I'm guessing?
Maybe make it a global constant value like webGLSupportedVariants
?
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.
WebGL traces different mechanism to render text i.e. via CanvasRenderingContext2D https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/font
At the moment only some of those options are supported.
So we should have it this way so that for those specific traces and subplots (gl2d, gl3d and mapbox) the variant
options are limited.
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.
Aha, this is because of WebGL I'm guessing?
Maybe make it a global constant value like
webGLSupportedVariants
?
Yes because of WebGL pipelines.
But no to have a global variable since scattergl
, gl2d
, gl3d
and mapbox
each uses different pipeline.
At one point we may be able to add one option to one not the other.
So I think it's better to keep it this way.
@@ -13243,10 +13308,18 @@ proto.update = function(bounds, labels, labelFont, ticks, tickFont) { | |||
if(!ticks[d][i].text) { | |||
continue | |||
} | |||
|
|||
var font = { | |||
family: ticks[d][i].font || tickFont[d].family, |
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.
why does this line say ticks[d][i]
instead of tickFont[d]
like the line below?
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.
This change is reflected from https://github.com/gl-vis/gl-axes3d/pull/24/files
gl-axes3d
has its own API and there was a bug there which didn't pick the right item.
This is fixed now in that PR.
stackgl_modules/package.json
Outdated
"gl-cone3d": "^1.6.0", | ||
"gl-error3d": "^1.0.16", | ||
"gl-heatmap2d": "^1.1.1", | ||
"gl-line3d": "1.2.1", | ||
"gl-mesh3d": "^2.3.1", | ||
"gl-plot2d": "^1.4.5", | ||
"gl-plot2d": "github:gl-vis/gl-plot2d#ac02a12ec9fa5283a79c042b900dfd0707c43891", |
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.
stackgl_modules/package.json
Outdated
@@ -13,15 +13,16 @@ | |||
"box-intersect": "plotly/box-intersect#v1.1.0", | |||
"convex-hull": "^1.0.3", | |||
"delaunay-triangulate": "^1.1.6", | |||
"gl-axes3d": "github:gl-vis/gl-axes3d#dae56c2e455a3f754f7dfe9aa74389a109df08a9", |
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.
stackgl_modules/package.json
Outdated
"gl-plot3d": "^2.4.7", | ||
"gl-pointcloud2d": "^1.0.3", | ||
"gl-scatter3d": "^1.2.3", | ||
"gl-scatter3d": "github:gl-vis/gl-scatter3d#9504c1bff7d9cfed8a85866cbc5e47cb32bdfb9a", |
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.
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.
Unicase looks terrible 🙈 Is that how it's supposed to look? The rest look fine haha.
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.
Yes it should look just like that.
I kind of like it ;)
💪 💪 💪 This is really awesome @archmoj . 💯 Huge improvement for Plotly.js. No more blocking comments from me. IMO it would be ideal to have a few more tests which set a font style/weight/variant for just part of the plot rather than just setting the global layout font, e.g. by using |
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.
🚀
Resolves #4646.
Support various font styling options all over the place 🎉 Enjoy!
@plotly/plotly_js
cc: @ndrezn
TODO:
scattergl
scatter3d
heatmapgl
mapbox
variant
options in gl3d, gl2d and regl tracesNo longer needed:
vectorize-text
: AddfontStretch
option mikolalysenko/vectorize-text#22vectorize-text
in stackgl modules - No longer needed asstretch
is not implemented.