d3plus / d3plus-text

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

textWrap issue with overflow and long text #111

Closed pdesoyres closed 5 years ago

pdesoyres commented 5 years ago

When running following code

import wrap from 'd3plus-text/es/src/textWrap';

const wrapper = wrap().overflow(true);
const result = wrapper('veryLongTextWithoutAnyLimitCharacterThatShouldBeTruncatedWhenUsingOverflow');
console.log(result);

I get an error : TypeError: Cannot read property 'replace' of undefined at https://github.com/d3plus/d3plus-text/blob/46be26331a0b88ed15afe8b8e857f1aa71a48f3a/src/textWrap.js#L59

davelandry commented 5 years ago

@pdesoyres thanks for finding this. I've pushed a fix, please update your d3plus-text install and let me know if it works for you!

Also, did you know you can simplify your ES module import?

import {textWrap} from 'd3plus-text';
pdesoyres commented 5 years ago

@davelandry thank you for this quick fix. Now, I have no error anymore.

But, is there a way to truncate the text and add an ellipsis at the end ?

davelandry commented 5 years ago

Setting .overflow(true) will allow strings that are longer than the defined .width( ) property to not be truncated. If you set .overflow(false) (the default), you will see you result object containing truncated: true and an empty lines array.

The textWrap function does not handle adding an ellipsis to the end of the truncated lines. We made a stylistic decision to rely on the renderer to handle that, so in d3plus it's handled in the TextBox class: http://d3plus.org/examples/d3plus-text/getting-started/

pdesoyres commented 5 years ago

Thank you @davelandry . I will handle ellipsis with TextBox.

Anyway, I found another bug at the same line of code : Running following code

import {textWrap} from 'd3plus';

const wrapper = textWrap().overflow(true);

const longWord = 'veryLongTextWithoutAnyLimitCharacterThatShouldBeTruncatedWhenUsingOverflow';

const result = wrapper(`${longWord} ${longWord}`);
console.log(result);

will throw the same Error : TypeError: Cannot read property 'replace' of undefined

I suggest to fix that be changing https://github.com/d3plus/d3plus-text/blob/3dddf7e8abca71f574e21590ebf6c5ee84b6f6af/src/textWrap.js#L59 by

if (lineData.length >= line) lineData[line - 1] = trimRight(lineData[line - 1]);
davelandry commented 5 years ago

@pdesoyres good catch! just pushed another release

pdesoyres commented 5 years ago

@davelandry thank you. This is now working fine.

ryancwalsh commented 4 years ago

@davelandry Thank you so much for providing this library! :-)

I'm currently trying to modernize the way my organization handles JS dependencies, and I'm having trouble with d3plus.

I figured I'd be able to use import d3plus from 'd3plus-text' and then be able to call functions like:

d3plus.textWidth(name, {'font-size': 12})

I haven't found documentation about how to use import with d3plus. I only see this and "You can also load directly from d3plus.org: <script src="https://d3plus.org/js/d3plus-text.v0.9.full.min.js"></script>"

I've tried all sorts of variations of that import line.

The error I'm getting is:

Uncaught TypeError: Cannot redefine property: innerHTML
    at Function.defineProperty (<anonymous>)
    at app.js:86569
    at app.js:86608
    at app.js:85587
    at Object.<anonymous> (app.js:85588)
    at Object.67.d3-array (app.js:87327)
    at o (app.js:1)
    at app.js:1
    at app.js:87536
    at Object.68.d3-array (app.js:87539)

If you have a moment, I'd very much appreciate any hints.

Thanks!

davelandry commented 4 years ago

@ryancwalsh great to see some ES6 users! each d3plus package exports every Class and Function as named exports, so you should be able to do the following:

import {textWidth} from "d3plus-text";
ryancwalsh commented 4 years ago

@davelandry Thanks so much for your response!

And that's one of the variations I tried (and I just tried again since you confirmed that that was the right approach).

Unfortunately I get the error that I showed above.

What's weird is that I can get everything to work if I put <script src="https://d3plus.org/js/d3plus-text.v0.9.full.min.js"></script> in my HTML instead.

I will keep playing around. Maybe I'll be able to narrow down what the problem is.

ryancwalsh commented 4 years ago

@davelandry Upgrading from "0.9.43" to "^0.10.1" fixed it.

The other way I noticed I could fix it (temporarily within 0.9.43) was by adding configurable: true to two places that Object.defineProperty had been called (https://stackoverflow.com/a/25518045/470749).

ghost commented 3 years ago

Hi. We have recently upgraded to Babel7 and are hitting an issue 'TypeError: TypeError: Cannot redefine property: innerHTML' in d3plus-text. We have various d3 packages installed as dependencies ... "d3-array": "^2.8.0", "d3-selection": "^2.0.0", "d3-svg-annotation": "^2.5.1", "d3-transition": "^2.0.0", "d3plus-text": "^0.10.1", ...

We get the same issue with 0.9.25 version.

Inserting "configurable: true" at the points in d3plus-text.js where it defines the innerHTML property fixes the issue temporarily

    Object.defineProperty(SVGElement.prototype, 'innerHTML', {
          configrable: true,
      get: function () {
          ...

Is there a fix for this?

davelandry commented 3 years ago

@phullphill two questions:

The docs of defineProperty state that configurable should be set to true "if the type of this property descriptor may be changed and if the property may be deleted from the corresponding object."

So because that makes it work, it makes me think you're build process is loading both the d3plus polyfill for innerHTML before then loading another polyfill for innerHTML on top of it (causing the need for it to be changed and/or deleted).

I've been using d3plus-text with Babel7 for a while now with no issue, and I believe if you import the d3plus-text functions as ES modules then it shouldn't include the d3plus polyfill.