Skip to content

Adding || to alt tag causes a false positive #2

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

Closed
jamsea opened this issue Mar 29, 2016 · 4 comments
Closed

Adding || to alt tag causes a false positive #2

jamsea opened this issue Mar 29, 2016 · 4 comments
Assignees
Labels

Comments

@jamsea
Copy link

jamsea commented Mar 29, 2016

Currently getting a false positive with this block of code:

<img
    alt={briefHeadline || "Player thumbnail"}
    src={`${posterImage}`}
    style={styles.img}
/>

Even though this passes:

<img
    alt={briefHeadline}
    src={`${posterImage}`}
    style={styles.img}
/>

This unfortunately means there's a chance briefHeadline could be null/undefined preventing an alt tag from being rendered. Any ideas of what could be wrong?

@beefancohen beefancohen self-assigned this Mar 29, 2016
@beefancohen
Copy link
Contributor

Hey @jamsea - thanks for reporting this. Extracting the value of prop types is a hard problem. Static analysis tools (such as eslint) can't determine actual values of variables until runtime, so a linter cannot pick up whether a briefHeadline is null or undefined. We can use react-a11y for that because this tool runs when running your app.

However, the first use case should not throw an error now in 0.4.3 - the code just didn't handle the case where a prop was a logical expression. So this should now be fixed if you reinstall.

I hope to in the near future extract all of my utils for parsing the JSX nodes to determine existence/truthy values into separate modules so that I can better implement and test all cases for what can be used as a prop (and so others can use to build awesome static analysis tools of props, as well!)

Let me know if you see this issue persist and please continue to file if you find more, it's super helpful! 💯

@jamsea
Copy link
Author

jamsea commented Mar 31, 2016

Great, this is working now 👍 I'm in the middle of a big accessibility migration and this tool and https://github.com/angular/protractor-accessibility-plugin have both been life savers. Thanks for your hard work! I'll let you know if I find any other issues

@jamsea jamsea closed this as completed Mar 31, 2016
@beefancohen
Copy link
Contributor

@jamsea ahh dude that is awesome, never seen that before. Just led me to this: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules

Will use this as a reference to expand this project.

Glad to see you are focusing on accessibility :)

@jamsea
Copy link
Author

jamsea commented Mar 31, 2016

Great! Create issues for the rules you're working on. I might be able to help out. We have a team of about 18 front end engineers, so any issue we can catch at the linter level saves us time. That way we don't have to wait for protractor to run our entire test suite to catch violations after we check in code.

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

No branches or pull requests

2 participants