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

Implement blur effect #9

Closed
wants to merge 2 commits into from
Closed

Implement blur effect #9

wants to merge 2 commits into from

Conversation

chregu
Copy link
Contributor

@chregu chregu commented Nov 28, 2017

This patch (currently only) implements the blur effect. Maybe useful for some. If anyone likes to add more effects, I'd be happy to integrate them here, before it gets eventually merged.

@leofeyer
Copy link
Member

I like the idea and it could help solving contao/core-bundle#1202 (comment) – although we might need to implement the other methods, too.

@ausi
Copy link
Member

ausi commented Nov 28, 2017

Looks great, thank you @chregu!

I think we should add some checks here:

  1. If the filter attribute is already set, we should append or prepend the blur filter.
  2. IDs need to be unique, so we should check that the ID of the filter doesn’t exist already.
  3. SVGs can be inlined in HTML, so we should try to use a very unique id (maybe random or some hash).

@chregu
Copy link
Contributor Author

chregu commented Nov 28, 2017

Totally agree with those 3 points. It was sufficient enough for our needs, but should definitely be extended for common useage

@ausi ausi self-assigned this Nov 28, 2017
@ausi ausi mentioned this pull request Feb 25, 2018
@ausi
Copy link
Member

ausi commented Feb 25, 2018

I created a followup pull request: #10

@chregu It would be great if you could take a look if this still generates the desired results for your use case.

@ausi ausi closed this Feb 25, 2018
@chregu
Copy link
Contributor Author

chregu commented Feb 26, 2018

@ausi Cool, thanks a lot for the additional work you put into. Works fine on our project with your code.

@Toflar
Copy link
Member

Toflar commented Feb 26, 2018

@chregu, is that for rokka.io? 😄

@chregu
Copy link
Contributor Author

chregu commented Feb 26, 2018

@Toflar Sure thing ;)

ausi pushed a commit to ausi/imagine-svg that referenced this pull request Mar 1, 2018
ausi pushed a commit that referenced this pull request Mar 1, 2018
@ausi
Copy link
Member

ausi commented Mar 1, 2018

Merged in f126fca and ba853e2.

Thank you @chregu!

@chregu
Copy link
Contributor Author

chregu commented Mar 1, 2018

Thank you all for making it better 💛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants