Graph3d tooltip styling #2780
Graph3d tooltip styling #2780
Conversation
|
Thanks for your pull request! The network module uses a class name |
|
Well, my first thought was to just replace all hard coded styling altogether with a class, but then I realised that none of the Graph3D examples loads the bundled css file and doing such as thing would break things. Therefore I aimed for a more backwards compatible solution. But yes, for a new major version a better solution would be to introduce a Any chance that the current PR could be applied for a new minor release since it doesn't break things? |
|
I see - good point. Then this approach is absolute fine for a minor/major version update. Let's keep it as it is and we include it in the next minor release.
I would love to see another pull request, which adds the same feature with the class thing. We are then able to merge this into our |
lib/graph3d/Settings.js
Outdated
| /** | ||
| * Merge fields from src to dst | ||
| */ | ||
| function mergeFields(src, dst, fields) { |
There was a problem hiding this comment.
I think something like this already exists in the utils here.
There was a problem hiding this comment.
Done!. Used the util's methods instead
| document.getElementById('style').onchange = drawVisualization; | ||
| } | ||
| </script> | ||
| <script src="../googleAnalytics.js"></script> |
There was a problem hiding this comment.
As far as I know, we don't want googleAnalytics anymore (reference). Can you remove it?
There was a problem hiding this comment.
Yes please remove the analytics.
There was a problem hiding this comment.
Sorry, this was probably due to me branching of master initially... But this file is now removed altogether.
| showShadow: false, | ||
|
|
||
| // Option tooltip can be true, false, or a function returning a string with HTML contents | ||
| //tooltip: true, |
| @@ -0,0 +1,132 @@ | |||
| <!doctype html> | |||
There was a problem hiding this comment.
This example is basically just a copy of this one. Maybe it is better to adapt the other example for such a small change. What do you think?
There was a problem hiding this comment.
I think so too. Feel free to adopt the existing example to your needs.
There was a problem hiding this comment.
ok. removed "my" example and altered the original a bit
docs/graph3d/index.html
Outdated
| <td>tooltipStyle</td> | ||
| <td>Object</td> | ||
| <td>{<br> | ||
| content :<br>{ |
There was a problem hiding this comment.
Do we really need No-break spaces here?
docs/graph3d/index.html
Outdated
| <td>tooltipStyle</td> | ||
| <td>Object</td> | ||
| <td>{<br> | ||
| content :<br>{ |
| @@ -0,0 +1,132 @@ | |||
| <!doctype html> | |||
There was a problem hiding this comment.
I think so too. Feel free to adopt the existing example to your needs.
| document.getElementById('style').onchange = drawVisualization; | ||
| } | ||
| </script> | ||
| <script src="../googleAnalytics.js"></script> |
There was a problem hiding this comment.
Yes please remove the analytics.
| @@ -67,6 +67,29 @@ var DEFAULTS = { | |||
|
|
|||
| style : Graph3d.STYLE.DOT, | |||
| tooltip : false, | |||
|
|
|||
| tooltipStyle : { | |||
There was a problem hiding this comment.
I haven't tested it yet. Please make sure the existing tooltips are basically not changed, or we have to consider this as a breaking change.
There was a problem hiding this comment.
Tested all combos, if no tooltipStyle is provided the tooltips are not changed and are exactly as before.
PR for issue #2769
Graph3D's option can now take a 'tooltipStyle' object with styling properties for 'content', 'line' and 'dot'.