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

<br> in Node tooltips should be \n instead to render correctly in Chrome and Edge #251

Open
mac471 opened this issue Feb 24, 2019 · 8 comments

Comments

@mac471
Copy link

mac471 commented Feb 24, 2019

This is a new issue for an old problem: how text displays in tooltips. For "links", the two lines are separated by a newline character \n. For "nodes", the two lines are separated by a <br> tag.

While this works in Safari, it does not appear to work in Chrome or Edge. The <br> tag is just ignored and the two lines run together.

I believe the answer is simple. In the line below, just replace <br> with \n (like the way links are handled) to fix the problem. I've edited my local copy of sankeyNetwork.js and this now works on Safari, Chrome and Edge.

.html(function(d) { return "<pre>" + d.name + "<br>" + format(d.value) +

As an aside, I believe using <br> should work in Chrome and Edge. I've posted a question on StackOverflow to ask this question

Do you want me to make this change via pull request?

@cjyetman
Copy link
Collaborator

Ugh... this issue has caused way more hassle than anything else by far... all just to put a newline in tooltips. see #180 #152 #130 #146 #151 #175 #215 #250 just for starters.

The problem is, tooltips were not designed to be multiline, so browsers react differently to whatever hack you use to enable it. Then, SVG titles are not designed to contain HTML, so whatever hack we use to enable that causes various problems and conflicts. We've gone through multiple rounds of trying to get this working on the widest range of browsers, and spent an inordinate amount of time testing and researching options. Frankly, it pains me to imagine reviewing another change that might fix something for someone but break something for everyone else, particularly with this issue that has caused so much trouble before.

That being said, since the line for the links is...

.html(function(d) { return "<pre>" + d.source.name + " \u2192 " + d.target.name +
"\n" + format(d.value) + " " + options.units + "</pre>"; });

and the line for the nodes is...

.html(function(d) { return "<pre>" + d.name + "<br>" + format(d.value) +
" " + options.units + "</pre>"; });

it seems logical that they should be the same. The main significant difference between the nodes and links is that one is a SVG rect and one is a SVG path, and I can't imagine why that would matter. There's another pending PR #215 that I will hopefully merge soon that may conflict with this change though... he was appending text to the pre object instead of html, hence the \n instead of a <br>.

@cjyetman
Copy link
Collaborator

and there's some report here #130 (comment) that suggests \n is not necessarily the most compatible option

@mac471
Copy link
Author

mac471 commented Feb 24, 2019

Seems like different browsers are rendering <br> differently so short of creating lots of browser-specific code, how about the following as a potential solution:

Add new arguments to the sankeyNetwork function that lets the caller specify the strings to use for delimiting between the node/link display name and the node/link value. For instance the argument nodeDelimiter defaults to "
" to maintain current compatibility and the argument linkDelimiter defaults to "\n". Those of us who don't really care about showing the tool tip on two lines can set these values to ": " so that the tool tip appears on a single line.

The code in sankeyNetwork.js would change to something like this:

For nodes:
.html(function(d) { return "<pre>" + d.name + options.nodeDelimiter + format(d.value) + " " + options.units + "</pre>"; });

For links:
.html(function(d) { return "<pre>" + d.source.name + " \u2192 " + d.target.name + options.linkDelimiter + format(d.value) + " " + options.units + "</pre>"; });

This way, you can avoid a rash of new bug reports from changing <br> to \n while giving developers the choice of how to handle the delimiter in both node and link tooltips. Probably the safest thing is to just show it all on the same line but in the spirit of not changing things that work the way people already want, set the defaults as suggested above.

@cjyetman
Copy link
Collaborator

My preference would be to make the default delimiter either ": " or " - " for full, safe compatibility. Then maybe add some functionality that would allow a user to set custom code for the tooltips, then they take full responsibility if they want to use <br> or "\n", and whatever the consequences are on their browser.

@mac471
Copy link
Author

mac471 commented Feb 24, 2019

So that's fine but I would use ": " instead of " - " to avoid confusion with the negative sign (even though negative values don't make sense in a Sankey).

If you go this route, then instead of adding the arguments I suggested above for backward compatibility, I would consider adding the ability to provide custom JavaScript functions to render the node and link tooltip text as desired - perhaps through the $options list?

@cjyetman
Copy link
Collaborator

also of note... using 0.4 release version and the 0.4.9000 dev version of networkD3 on macOS 10.14.3, R version 3.5.2, and RStudio version 1.2.1114... neither (links or nodes) of the line breaks in the tooltips work in the RStudio Viewer pane. Personally, I believe that the RStudio Viewer pane should be the primary target for compatibility because that's the most accessible output.

@cjyetman
Copy link
Collaborator

If you go this route, then instead of adding the arguments I suggested above for backward compatibility, I would consider adding the ability to provide custom JavaScript functions to render the node and link tooltip text as desired - perhaps through the $options list?

Yes, so for example, the defaults could be...
link_tooltip = 'd.source.name + " \u2192 " + d.target.name + " - " + format(d.value) + " " + options.units'
and
node_tooltip = 'd.name + " - " + format(d.value) + " " + options.units'

@mac471
Copy link
Author

mac471 commented Feb 26, 2019

This is probably the best alternative as it gives the caller the ability to specify what the node/link tooltips should look like while making sure that it at least works in RStudio.

Sounds like a plan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants