add repeat to radialGradient #2
Conversation
|
Apparently the notifications were not set correctly and I was not receiving any of them. We'll be on it now. |
| display:block; | ||
| } | ||
|
|
||
| div.psudo-after::before { |
There was a problem hiding this comment.
Typo likely, pr is pretty old now. I'll spend some time and clean up bits.
|
|
||
| this.path(path); | ||
| this.ctx.fillStyle = radialGradient; | ||
| this.path.bind({ctx, formatPath: this.formatPath.bind({ctx})})(normalisedPath); |
There was a problem hiding this comment.
The fact that you need to use bind seem suspicious. Can't we use arrow functions instead?
There was a problem hiding this comment.
I'll try take a look, this I believe was a workaround because background (html2canvas) didn't allow arguments again long time ago I'll take a look
|
@mikey0000 Can you illustrate the problem this is supposed to fix, maybe with some screenshots? Would be helpful. Thanks a lot! |
Should be a screenshot on the original pr? Tests should show how it works too? |
|
Anything that helps me understand a bit better the problem and the possible implications. |
|
So this is a link to the original PR niklasvh#2710 I'll re-pull this down and screenshot the test |
|
So looking over this, it resolves background-repeat: repeat-y; not being rendered. I'll take a look today and see if I can improve the code a bit. |
|
|
|
Ok, but what is the related css? And what is the current result |
2 participants
Summary
radial gradients do not repeat when they should
This PR fixes/implements the following bugs/features
Explain the motivation for making this change. What existing problem does the pull request solve?
adds the repeat functionality to the radial gradients.
Fixes niklasvh#2103
possibly fixes niklasvh#2620 niklasvh#2474
At this stage I know I need some feedback and direction as some of the changes aren't entirely tidy. Happy to discuss the direction you want to take this and planning some tests to cover it.