Skip to content

Conversation

@archmoj
Copy link
Contributor

@archmoj archmoj commented Nov 13, 2018

This PR enables the use of tags namely < b > < i > < sup > < sub> and < br > within webGL texts.
Please also refer to Plotly PR 3207 for user issues, examples & more information.
@mikolalysenko
@monfera

@mikolalysenko
Copy link
Owner

Could we get an option to make the styling stuff only run if you turn on a flag? Not sure how much stuff this would break if we put it in by default all the time.

Maybe have an option for "style" just like the newlines code.

@archmoj
Copy link
Contributor Author

archmoj commented Nov 16, 2018

I still need to setup few other things here.
PR Status: in progress

lib/vtext.js Outdated
bolds: false, // Note: could also be turned on as new default
italics: false, // Note: could also be turned on as new default
subscripts: false, // Note: could also be turned on as new default
superscripts: true // Note: superscripts used to be partly supported
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually didn't support superscripts in this module. Anyway, what do you think @mikolalysenko @etpinard about the default behaviour of styling tags? Should we keep all of them off? Or if you prefer to enable any or the rest by default?

@archmoj
Copy link
Contributor Author

archmoj commented Nov 16, 2018

@mikolalysenko
Your suggestion of having style options was very good.
FYI - I also disabled them by default and documented this in the readme.
Status of this PR (the vectorize-text module) is now reviewable.
With regards.

@mikolalysenko mikolalysenko merged commit cc5cb50 into mikolalysenko:master Nov 17, 2018
@archmoj archmoj deleted the issue-br3d branch November 17, 2018 20:49
@archmoj
Copy link
Contributor Author

archmoj commented Nov 17, 2018

Thanks @mikolalysenko for your review & the merge.
Once I tried to bump & release a minor version; I noticed that I don't have github & possibly npm permissions for this package.
I don't know if @bpostlethwaite @etpinard could also help me with this?

@mikolalysenko
Copy link
Owner

mikolalysenko commented Nov 18, 2018 via email

@archmoj
Copy link
Contributor Author

archmoj commented Nov 18, 2018

Thanks @mikolalysenko for the new release.

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