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

Add browserShadowBlurConstant to adjust shadowBlur value #4413

Merged
merged 3 commits into from
Oct 25, 2017

Conversation

melvynhills
Copy link
Contributor

@melvynhills melvynhills commented Oct 25, 2017

Expose browserShadowBlurConstant to adjust shadowBlur value
close #4402

@asturur
Copy link
Member

asturur commented Oct 25, 2017

would you mind check if there are some tests of setShadow in the object tests and in case add one that demostrate that this thing works?

i can guide you.

@melvynhills
Copy link
Contributor Author

Yeah in fact I tried to run the tests by following the Readme but when running testem I had errors of QUnit missing, although I did npm install and even npm install qunit -g 😕

@asturur
Copy link
Member

asturur commented Oct 25, 2017

use npm run test testem is a change that got unfinished.

@melvynhills
Copy link
Contributor Author

I get this error when running npm run test :

screen shot 2017-10-25 at 14 58 22

@asturur
Copy link
Member

asturur commented Oct 25, 2017

remove that npm install quint -g that you did before and reinstall fabric?

@melvynhills
Copy link
Contributor Author

melvynhills commented Oct 25, 2017

What do you mean by "reinstall fabric" ?
What I did:

  • npm uninstall qunit -g
  • rm -rf node_modules
  • npm install
  • npm run build
  • npm run test

=> Still have the same error.

(I see you opened a PR specifically for this, I guess it should ring a bell?)

Then I just tried to patch QUnit with QUnit.ok = QUnit.assert.ok.bind(QUnit.assert);, that kinda worked but then I get another error (maybe the one caught by QUnit in the first place) :

{ Error: Image given has not completed loading
    at /fabric.js/test/unit/image.js:68:35
    at /fabric.js/test/unit/image.js:99:21
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:513:3) message: 'Image given has not completed loading' }

Note: I'm using Node v8.8.0

@melvynhills
Copy link
Contributor Author

Skipped all tests but unit/object.js so I could run the suite, because I have errors with any test involving fabric.Image.

Added a simple test, tell me if it's enough. Thanks

@asturur
Copy link
Member

asturur commented Oct 25, 2017

If you have trouble running image tests it does me bring to think you have problem installing node-canvas.

@asturur
Copy link
Member

asturur commented Oct 25, 2017

test is ok, i just added descriptions so that if it fails you know what failed. if i did not broke linting i m going merge right now.

@asturur asturur changed the title Expose browserShadowBlurConstant to adjust shadowBlur value (#4402) Add browserShadowBlurConstant to adjust shadowBlur value Oct 25, 2017
@asturur asturur merged commit 0ddf90c into fabricjs:master Oct 25, 2017
@asturur asturur mentioned this pull request Nov 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

shadow doesn't render the same in chrome vs safari
2 participants