Closed rhanb closed 6 years ago
Merging #42 into master will increase coverage by
0.46%
. The diff coverage is88.88%
.
@@ Coverage Diff @@
## master #42 +/- ##
==========================================
+ Coverage 92.85% 93.31% +0.46%
==========================================
Files 1 1
Lines 406 434 +28
==========================================
+ Hits 377 405 +28
Misses 29 29
Impacted Files | Coverage Δ | |
---|---|---|
dist/granim.js | 93.31% <88.88%> (+0.46%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update dcfd2a0...a524750. Read the comment docs.
Thank you for your investment, This feature is quite similar to this requested feature #32 (the user will have even more control than setting the position in degrees.)
Instead of setting the position with pixels:
{ x0: 6, x1: 9, y0: 98, y1: 876}
It would be better with percentages for responsive layout (in this order x0, y0, x1, y1):
{ x0: '10%', y0: '25%', x1: '50%', y1: '75%'}
To do that you will have to transcribe the percentage in pixels:
case 'custom':
return ctx.createLinearGradient(
this.x1 / 100 * parseInt(this.customDirection.x0.split('%')[0], 10),
this.y1 / 100 * parseInt(this.customDirection.y0.split('%')[0], 10),
this.x1 / 100 * parseInt(this.customDirection.x1.split('%')[0], 10),
this.x1 / 100 * parseInt(this.customDirection.y1.split('%')[0], 10)
);
break;
I will add more comment in the files changed.
@sarcadass thanks for taking the time to check this PR and your comments, I will apply the changes tomorrow or the day after ! 👍
Don't forget to explain this new option in the docs too :).
@sarcadass almost done, just missing the createLinearGradient
part. Will do that this afternoon
@sarcadass all done 👍
@rhanb The build has failed, with one of the new tests you implemented, can you fix that please? Furthermore there are still many spaces you added before parenthesis, you can check them on the files modified tab, can you please remove them?
After that it should be OK to merge to master and release a new version 🙂. Thank you.
Thank you @rhanb for your contribution, I will release a new version tomorrow. Have a good day 🙂.
@sarcadass don't forget to publish the new version of the package on npm
👍
@rhanb I'm currently working on a new major version with new features, I will publish your new feature on the new version this week. Haver a good day.
@sarcadass was trying to use the new customDirection
feature. Unfortunately the validation part fails because of how the split
function work and the following regexp fails when the value is 0
: /^\d+$/
.
This pull request is a proposal to allow
granim.js
consumers to use a custom direction instead of one of the following direction:diagonal
,left-right
,top-bottom
,radial
.Goal ?
Having direction options make it easy to have nice gradients. But some of us may want to have more control over the positions of the gradient. So, this PR allows to basically exposes the
createRadialGradient
function and to allow more customisation. See bellow to see how.How?
By adding:
custom
option: https://github.com/rhanb/granim.js/blob/master/lib/setDirection.js#L26customDirection
that should look like this:(https://github.com/rhanb/granim.js/blob/master/lib/Granim.js#L11)
validateCustomDirection
function that allows to validate thecustomDirection
object if the consumer set the propertydirection
tocustom
. (instantiation
and fromchangeDirection
function) : https://github.com/rhanb/granim.js/blob/master/lib/validateCustomDirection.jsFeel free to ask any questions 👍