Skip to content
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

Link style #1681

Merged
merged 5 commits into from
May 15, 2017
Merged

Link style #1681

merged 5 commits into from
May 15, 2017

Conversation

alexcjohnson
Copy link
Collaborator

Fixes #1674

  • Make sure links in svg text get cursor: pointer like regular html links
  • Allow style for <a> elements in text, and make our parsing of these fields more robust
  • If all you have is a link, in annotation text, turn the whole text box into a clickable link

@etpinard OK?

@etpinard etpinard added status: reviewable bug something broken labels May 12, 2017
// so we need to use dy along with the uber hacky shift-back-to
// baseline below
sup: 'font-size:70%" dy="-0.6em',
sub: 'font-size:70%" dy="0.3em',
b: 'font-weight:bold',
i: 'font-style:italic',
a: '',
a: 'cursor:pointer',
Copy link
Contributor

Choose a reason for hiding this comment

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

👌

@@ -254,6 +260,16 @@ var UNICODE_TO_ENTITY = Object.keys(stringMappings.unicodeToEntity).map(function

var NEWLINES = /(\r\n?|\n)/g;

var SPLIT_TAGS = /(<[^<>]*>)/;
Copy link
Contributor

Choose a reason for hiding this comment

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

🐎

@@ -120,7 +122,8 @@ describe('svg+text utils', function() {
assertAnchorLink(node, 'mailto:support@plot.ly');
});

it('wraps XSS attacks in href', function() {
it('drops XSS attacks in href', function() {
// "XSS" gets interpreted as a relative link (http)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@@ -283,7 +283,7 @@ describe('Test sort transform interactions:', function() {

function wait() {
return new Promise(function(resolve) {
setTimeout(resolve, 60);
setTimeout(resolve, 100);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@etpinard it took 3 tries to get this test to pass on CI with the original timeout, first try with it a little longer.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

// Style and href: pull them out of either single or double quotes.
// Because we hack in other attributes with style (sub & sup), drop any trailing
// semicolon in user-supplied styles so we can consistently append the tag-dependent style
var STYLEMATCH = /(^|[\s"'])style\s*=\s*("([^"]*);?"|'([^']*);?')/i;
Copy link
Contributor

Choose a reason for hiding this comment

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

image

learn something new every day.

Copy link
Contributor

@rreusser rreusser May 15, 2017

Choose a reason for hiding this comment

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

(PSA: have to be careful with g! it adds a dependency on internal state of a regexp, which is super confusing if you're not expecting it)

@etpinard
Copy link
Contributor

Looks good 💃

@alexcjohnson alexcjohnson merged commit ad65c23 into master May 15, 2017
@alexcjohnson alexcjohnson deleted the link-style branch May 15, 2017 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants