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 src value to $error image if it is available - Fixes #65 #67

Merged
merged 2 commits into from
Sep 14, 2015

Conversation

dudeOFprosper
Copy link
Contributor

Currently, the src tag in issue #65 is hard coded to always show ".." for the source. This PR will add the the src to the tag if it is available.

@@ -18,6 +18,7 @@ class AltTextPlugin extends Plugin {

reportError(el) {
let $el = $(el);
let src = $el.prop("src") || "..";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the failing image in test.html to have a relative src (i.e. src="jdan.png"), we see the following:

image

This is somewhat problematic as it does not accurately reflect the contents of the src attribute. The reason for this is because jQuery's .prop() will resolve any URLs using the following process.

.attr() will return the actual contents of the src attribute, can we use that instead? You can even do el.getAttribute("src") and skip jQuery all together.

@jdan
Copy link
Owner

jdan commented Sep 14, 2015

Hey @dudeOFprosper,

Thanks for tackling this one so quickly. Left a brief comment inline about .prop() if you have some time to do a follow-up. Happy to answer any questions!

Jordan

@jdan
Copy link
Owner

jdan commented Sep 14, 2015

(and thank you for signing the CLA)

jdan added a commit that referenced this pull request Sep 14, 2015
Add src value to $error image if it is available - Fixes #65
@jdan jdan merged commit 26e38c5 into jdan:master Sep 14, 2015
@jdan
Copy link
Owner

jdan commented Sep 14, 2015

Awesome, thanks so much! 🌟 My apologies for the confusing JSX + ES6 template strings going on in there.

@dudeOFprosper
Copy link
Contributor Author

@jdan thanks for the advice! I went with prop, just didn't like the $el[0].getAttribute("src") vs $el.attr("src"). But I could see arguments for either.

@jdan
Copy link
Owner

jdan commented Sep 14, 2015

@dudeOFprosper el is there, too (then let $el = $(el); fwiw :)

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