-
Notifications
You must be signed in to change notification settings - Fork 25
fix: Contrast (Minimum) (WCAG SC 1.4.3) #817
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
Conversation
✅ Deploy Preview for cld-video-player ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cld-vp-esm-pages ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
||
| // Add TitleBar as default | ||
| children.push('titleBar'); | ||
| if (children.indexOf('titleBar') === -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need this "if" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was playing with colors opacity, and got weird results visually, this is how I found out the title is being rendered twice :)
This if prevents that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ShayLevi could this be what's failing the test here? I couldn't find any reason why it would fail, the videos obviously play fine in the deploy preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tsi is it possible that the video element id was changed? I see we have failures on multiple players page and colors API page due to that.
This PR fixes a few text color-contrast issues
Before:

After:
