throw when container width is auto

[futurist]

Below line:

https://github.com/TehShrike/get-text-fit-size/blob/a61cd31cd85c80a06e9d4d2ad10fe7039331d645/index.js#L11

will throw if the containerElement has a width: 'auto', since pxRegex().exec(str) will be null

[futurist]

And This won't work if you are using box-sizing: border-box, even the width is not auto.

https://stackoverflow.com/questions/25197184/get-the-height-of-an-element-minus-padding-margin-border-widths

[TehShrike]

oooh! Good catch. Would you be willing to add a test or two? Adding a new container element with different styles to the test html would work fine.

What should be returned when the container has an auto width? Maybe it should actually be an error, but a specifically thrown one with a reasonable error message?

I'm not sure about the implications of border-box. I suppose that needs a test as well.

[futurist]

When setting box-sizing: border-box to container, the width value will contain border and padding, which will shrink the actual content width space, the link above have an example and explanation, please check.

I've checked out your lib, npm it, got an error, I don't know how to use your test, since cannot add any.

$ npm it

npm WARN enoent ENOENT: no such file or directory, open '/Users/pro/github/get-text-fit-size/node_modules/_@juliangruber_tap-finished@0.0.2@@juliangruber/package.json'
npm WARN enoent ENOENT: no such file or directory, open '/Users/pro/github/get-text-fit-size/node_modules/_@types_node@7.0.52@@types/package.json'

> get-text-fit-size@1.0.3 test /Users/pro/github/get-text-fit-size
> browserify test.js | tape-run

Error: Cannot find module '../../../../_is-buffer@1.1.6@is-buffer/index.js' from '/Users/pro/github/get-text-fit-size/node_modules/_core-util-is@1.0.2@core-util-is/lib'
    at /Users/pro/github/get-text-fit-size/node_modules/_resolve@1.1.7@resolve/lib/async.js:55:21
    at load (/Users/pro/github/get-text-fit-size/node_modules/_resolve@1.1.7@resolve/lib/async.js:69:43)
    at onex (/Users/pro/github/get-text-fit-size/node_modules/_resolve@1.1.7@resolve/lib/async.js:92:31)
    at /Users/pro/github/get-text-fit-size/node_modules/_resolve@1.1.7@resolve/lib/async.js:22:47
    at FSReqWrap.oncomplete (fs.js:114:15)
internal/streams/legacy.js:59
      throw er; // Unhandled stream error in pipe.
      ^

Error: javascript required
    at Stream.<anonymous> (/Users/pro/github/get-text-fit-size/node_modules/_browser-run@4.1.0@browser-run/index.js:35:34)
    at _end (/Users/pro/github/get-text-fit-size/node_modules/_through@2.3.8@through/index.js:65:9)
    at Stream.stream.end (/Users/pro/github/get-text-fit-size/node_modules/_through@2.3.8@through/index.js:74:5)
    at Stream.method [as end] (/Users/pro/github/get-text-fit-size/node_modules/_duplexer@0.1.1@duplexer/index.js:47:39)
    at Stream.<anonymous> (/Users/pro/github/get-text-fit-size/node_modules/_throughout@0.0.0@throughout/index.js:7:25)
    at _end (/Users/pro/github/get-text-fit-size/node_modules/_through@2.3.8@through/index.js:65:9)
    at Stream.stream.end (/Users/pro/github/get-text-fit-size/node_modules/_through@2.3.8@through/index.js:74:5)
    at Stream.onend (internal/streams/legacy.js:44:10)
    at emitNone (events.js:91:20)
    at Stream.emit (events.js:188:7)
npm ERR! Test failed.  See above for more details.

[TehShrike]

Huh, there must have been an issue while installing dependencies.

After poking at the folder on my machine to verify that it still worked for me, I noticed that there were some uncommitted changes to the package-lock file. So, try pulling the latest version from master, deleting node_modules, and see if npm i works successfully after that.

Once the install completes and the tests run, you can add new tests by editing https://github.com/TehShrike/get-text-fit-size/blob/master/test.js - it's pretty lightweight.

[futurist]

Now npm i passed. But the network is blocked...(I'm in China)

mac:get-text-fit-size pro$ cnpm it

> electron@1.6.10 postinstall /Users/pro/github/get-text-fit-size/node_modules/electron
> node install.js

/Users/pro/github/get-text-fit-size/node_modules/electron/install.js:47
  throw err
  ^

Error: connect ETIMEDOUT 52.216.17.216:443
    at Object.exports._errnoException (util.js:1050:11)
    at exports._exceptionWithHostPort (util.js:1073:20)
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1097:14)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! electron@1.6.10 postinstall: `node install.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the electron@1.6.10 postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

Here you even cannot install electron form npm....

Cannot help now, please add test for your own, if it's easy.

[TehShrike]

Oh snap, I didn't know electron was blocked there!

Try npm test:firefox or npm test:chrome, those should test locally using your locally installed browsers.

https://github.com/TehShrike/get-text-fit-size/blob/ef05d7a9b24a89d63deeb573dbb87a4bd5c44102/package.json#L9-L10

[TehShrike]

I'm hoping to get you to help me, in part because I would love more contributors, and in part because I'm not sure what the correct behavior should be in the border-box situation 😅

[TehShrike]

@futurist let me know if you experience any issues running the browser tests locally

[futurist]

@TehShrike Please review the PR #3 , But I think the test should notify why the test failed, just like console.error on browser side. Now, in the test, I cannot see anything valuable message about why the test failed.

[TehShrike]

I updated the assertion messages to be a bit clearer.

Before: not ok 6 93.56716978844639 less than 62.08 and greater than 61.58

After: not ok 11 93.56716978844639 should be less than 62.08