d3plus / d3plus-text

A smart SVG text box with line wrapping and automatic font size scaling.
MIT License
105 stars 18 forks source link

no text appearing in subsequent renders on Windows #76

Closed IPWright83 closed 6 years ago

IPWright83 commented 6 years ago

With the font resizing on, if there isn't enough space instead of adding ellipsis on it simply doesn't add any text.

Expected Behavior

Ellipsis if there isn't space for the text. In this provided example I would expect the last 3 characters that can be rendered to be missed off ABCDEFGHIJKLMNOPQRSTUVWXY...

Current Behavior

No text is added to the DOM

Steps to Reproduce (for bugs)

https://jsfiddle.net/IPWright83/uscw0Lh6/1/

I'm using the following where radius = 150 and I'm using 2 strings: ABCDEFGHIJKLMNOPQRSTUVWXYZ12 and ABCDEFGHIJKLMNOPQRSTUVWXYZ123

new d3plus.TextBox()
    .data([d])
    .select(elements[i])
    .fontResize(true)
    .height(radius)
    .width(radius)
    .text(d => d.name)
    .x(d => -radius / 2)
    .y(d => -radius / 2)
    .ellipsis((text, line) => line ? text.replace(/\.|,$/g, "") + "..." : "")
    .verticalAlign("middle")
    .textAnchor("middle")
    .render();  
  });
IPWright83 commented 6 years ago

@davelandry This is something I'd like to help out fixing, but I probably need a little guidance on first. I found it a little difficult to follow all the internals and understand what they were actually trying to do.

davelandry commented 6 years ago

We actually chose to switch to this behavior, because in use testing in visualizations we found that showing no text was more desirable than to show ... all over the place.

In your example, you could restore the old behavior by changing your ellipsis function to the following:

.ellipsis(text => text.replace(/\.|,$/g, "") + "...")
IPWright83 commented 6 years ago

@davelandry I still don't find the ellipsis function working. Could you take a look see what I'm doing wrong? https://jsfiddle.net/IPWright83/uscw0Lh6/2/

davelandry commented 6 years ago

@IPWright83 that example jsfiddle looks right to me. What are you expecting to happen? It is resizing the string to fit within the circles.

on a different note, have you seen the d3plus-shape module? makes drawing those circles a lot easier: https://jsfiddle.net/Lz9naLws/

IPWright83 commented 6 years ago

The behavior of the fiddle I posted... The first circle has the reduced size text (great!) but the second circle is just blank just as it was before modifying the ellipsis function. I need to find a way to render some text and ellipsis in that 2nd circle.

I haven't seen the d3plus-shape module, but honestly I prefer the more verbose d3 approach as I know exactly what's going on!

davelandry commented 6 years ago

@IPWright83 what is your browser and OS? I see both lines of text in your fiddle using Chrome and macOS:

screen shot 2018-02-06 at 10 02 27 am
IPWright83 commented 6 years ago

Interesting, I'm in Chrome 64bit Windows 10. Definitely different to what I'm seeing. I'll take a deeper look tomorrow amd update here.

IPWright83 commented 6 years ago

Took me a while to take a look at this. There must be something different between the measurement in Chrome on Windows because the text isn't even in the DOM for me:

image

The output is also consistent for me - FireFox, Edge, Chrome all exhibit exactly the same behavior.

davelandry commented 6 years ago

@IPWright83 very strange. Would you mind try running this fiddle and share the output and console messages? I think you're right in thinking it might be a different with DOM management.

IPWright83 commented 6 years ago

@davelandry Sure - so the output is the same as the image I shared with you before. The console output is as follows:

image

The content isn't copying well as a string unfortunately. I logged the outer HTML of each <g> after the TextBox function had ran:

image

IPWright83 commented 6 years ago

@davelandry Just wondering - anything more I can do to help out here?

davelandry commented 6 years ago

@IPWright83 clone the repo and start hacking! 😜

but seriously: if I were to start looking somewhere, I would probably start by testing the textWidth function here: https://github.com/d3plus/d3plus-text/blob/master/src/textWidth.js

IPWright83 commented 6 years ago

@davelandry Yeah sure, I've already got a clone. As per my 2nd comment on the issue - it's quite difficult trying to work out what the internals are actually trying to achieve, hence me not having looked at this already.

Appreciate the suggestion of looking at textWidth though.

IPWright83 commented 6 years ago

So I've started looking at this again, managed to put aside a couple of days all being well. Interesting thing to note firstly is that the data join inside TextBox.js to the boxes variable yields an empty selection the 2nd time through (the circle that displays no text).

davelandry commented 6 years ago

@IPWright83 I removed the word truncation, as it was producing some unexpected results in further testing. I may have misunderstood your PR, but we don't want strings like "China" being truncated to "C" or even "C..." when there is very little space. It becomes very confusing to not see the entire word.

Please confirm that this specific issue, subsequent renders on Windows, is still fixed with the latest release.

IPWright83 commented 6 years ago

Hi @davelandry - can I just check, what you expect to see? I find that if I have a single word e.g. "China" that's too long - then what happens is it'll just display nothing (which looks like something broke). In our case we have words that may be quite long "Applications" and we need something there e.g. "App..." to give an indication that there is a value there - you just might not be able to see the entire thing. What sort of unexpected results were you seeing? As this logic only kicks in if there's a single word I believe - to ensure that something renders.

davelandry commented 6 years ago

@IPWright83 in that case, I would make it an optional parameter. In the case of Bar Charts, we were seeing a lot of "B...", "A...", "C..." values, and that is not desired.

IPWright83 commented 6 years ago

@davelandry aah I see what you mean.

I did start down that route originally. I was going to use whether the ellipsis function had been defined to dictate whether or not text should be not visible, or truncated. But discovered it was always defined so changed my mind on that one.

I'll have to take a look at another point in time (and fix our version to .29) for now, unless you're able to revert/stick an extra flag on it? I guess a truncateSingleWord(bool)?

davelandry commented 6 years ago

@IPWright83 how about just truncateWord(bool)?

IPWright83 commented 6 years ago

@davelandry Sure - I'm pretty easy. This is your API and as long as it allows a single truncated word I'm happy! :D

IPWright83 commented 6 years ago

@davelandry Just to check - where you planning on adding truncateWord(bool) to the interface?

davelandry commented 6 years ago

@IPWright83 apologies for not being clear enough. It shouldn't be too hard to add (pretty much just putting your original logic behind a boolean), but I'm pretty swamped right now. I've made this separate issue for one of us to grab when either of us has the time. 👍

IPWright83 commented 6 years ago

@davelandry thanks! Yeah, I understand being swamped! I just realized our latest build had the potential to break as a result of an upgrade of d3plus-text which suddenly reminded me.

I'll try and get on it at some point.