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

Using binding in 'src' attribute instead of interpolation to support SafeUrl | SafeResourceUrl #28

Merged
merged 1 commit into from
Jan 16, 2019

Conversation

tomsjer
Copy link

@tomsjer tomsjer commented Jan 14, 2019

Hey, thanks for this lib, its really useful.
I know this PR is a duplicate of #13,
but I happen to need this little change as soon as possible.

I'm currently using the branch feature/base64 of my fork, to which I've pushed the dist/ dir in order to consume it from my package.json as so:
{ ..., "ngx-image-zoom": "https://github.com/tomsjer/ngx-image-zoom.git#feature/base64", ... }
Not so good... anyway, I've tested it with [src]="data:image/jpg;base64,/..." and [src]="http://placehold.it/200/200", both working ok.
Cheers.

@wittlock
Copy link
Owner

Hi, thanks for the pull request. This looks like an extremely tidy solution to this issue. I'll try to merge and push a new version to npm tomorrow with this, to get it to you speedily.
Do you think this would also fix issue #1 ? I think the lack of SafeUrl was the source of those issues as well. I'll test it out myself eventually though to be able to close the issue, just a late night thought. :)

So keep an eye out for 0.3.4 tomorrow. And hopefully more changes in the next couple of weeks.

@wittlock wittlock self-assigned this Jan 14, 2019
@tomsjer
Copy link
Author

tomsjer commented Jan 15, 2019

Hey! Thanks for the quick response :)

I don't know about #1, but I'll check it out first thing tomorrow morning at the office, where I'm actually using ngx-image-zoom.
I guess it should do it unless is some angular-specific bug, but I'll let you know.
Cheers!

@wittlock wittlock merged commit dff4103 into wittlock:master Jan 16, 2019
@wittlock
Copy link
Owner

@tomsjer 0.3.4 should be on npm with this stuff merged now. Let me know if something went wrong. It has been a little while since I did a release so I'm a bit rusty. :)

@tomsjer
Copy link
Author

tomsjer commented Jan 17, 2019

@wittlock , sorry about the delay.
Just tried the 0.3.4 release, working fine for me! Thanks!

BTW, I can confirm the following URI encoded SVG is working fine:
src="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='30' height='30' viewBox='0 0 30 30'%3E%3Cpath fill='%239ACA3C' fill-rule='evenodd' d='M15 0C6.75 0 0 6.75 0 15s6.75 15 15 15 15-6.75 15-15S23.25 0 15 0zm0 4.5c2.55 0 4.5 1.95 4.5 4.5s-1.95 4.5-4.5 4.5-4.5-1.95-4.5-4.5 1.95-4.5 4.5-4.5zm0 21.3c-3.75 0-7.05-1.95-9-4.8 0-3 6-4.65 9-4.65S24 18 24 21c-1.95 2.85-5.25 4.8-9 4.8z'/%3E%3C/svg%3E%0A"

Encoder tool: https://yoksel.github.io/url-encoder/

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.

2 participants