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

fix: Contrast (Minimum) (WCAG SC 1.4.3) #817

Merged
merged 3 commits into from
Feb 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/colors.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ <h3 class="text-primary">Player 1</h3>
data-cld-source='{ "publicId": "dog", "info": { "title": "Sea turtle", "subtitle": "Short movie about a sea turtle" } }'
data-cld-colors='{ "base": "#0b602b", "accent": "#cddc39", "text": "#fff" }'
class="cld-video-player cld-fluid"
id="player-1"
></video>
</div>
<h3 class="text-primary">Player 2</h3>
Expand All @@ -68,6 +69,7 @@ <h3 class="text-primary">Player 2</h3>
data-cld-source='{ "publicId": "sea_turtle", "info": { "title": "Sea turtle", "subtitle": "Short movie about a sea turtle" } }'
data-cld-colors='{ "base": "#111", "accent": "#03a9f4", "text": "#ffeb3b" }'
class="cld-video-player cld-fluid cld-video-player-skin-dark"
id="player-2"
></video>
</div>

Expand All @@ -78,6 +80,7 @@ <h3 class="text-primary">Player 3</h3>
playsinline
data-cld-source='{ "publicId": "elephants", "info": { "title": "Elephants", "subtitle": "Short movie about elephants" } }'
data-cld-colors='{ "base": "#ffffff", "accent": "#676a46", "text": "#007d29" }' class="cld-video-player cld-fluid cld-video-player-skin-light"
id="player-3"
></video>
</div>

Expand Down
11 changes: 7 additions & 4 deletions docs/multiple-players.html
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ <h3 class="text-primary">Player 1</h3>
playsinline
data-cld-public-id="elephants"
class="cld-video-player cld-video-player-skin-dark"
id="player-1"
></video>
</div>

Expand All @@ -64,8 +65,9 @@ <h3 class="text-primary">Player 2</h3>
muted
playsinline
data-cld-public-id="marmots"
class="cld-video-player cld-video-player-skin-light">
</video>
class="cld-video-player cld-video-player-skin-light"
id="player-2"
></video>
</div>

<h3 class="text-primary">Player 3</h3>
Expand All @@ -74,8 +76,9 @@ <h3 class="text-primary">Player 3</h3>
muted
playsinline
data-cld-public-id="snow_deer_short"
class="cld-video-player">
</video>
class="cld-video-player"
id="player-3"
></video>
</div>

<p class="mt-4">
Expand Down
3 changes: 3 additions & 0 deletions src/assets/styles/components/title-bar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
padding: clamp(1.3em, 4cqw, 2em);
pointer-events: none;
container-type: inline-size;
text-shadow: 0 0 0.5em var(--color-base);
color: var(--color-text);
background-image: linear-gradient(color-mix(in srgb, var(--color-base) 70%, transparent), transparent 100%);

.vjs-title-bar-title {
width: 100%;
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/chapters/chapters.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@
transform: translateX(-50%);
bottom: 2.7em;
position: absolute;
text-shadow: 0 0 4px color-mix(in srgb, var(--color-base) 40%, transparent);
padding: 0 0.3em;
font-weight: bold;
text-shadow: 0 0 8px var(--color-base), 0 0 1px var(--color-base), 0 0 1px var(--color-base);
&:not(:empty) ~ .vjs-vtt-thumbnail-display {
bottom: 4em;
}
Expand Down
5 changes: 0 additions & 5 deletions src/plugins/colors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ const playerColors = `
color: rgba(--text-color, 0.3);
}

.PLAYER-CLASS-PREFIX .vjs-title-bar {
color: --text-color;
background-image: linear-gradient(rgba(--base-color, 0.4), rgba(255, 255, 255, 0) 100%);
}

.PLAYER-CLASS-PREFIX .vjs-recommendations-overlay {
color: --text-color;
background-color: rgba(--base-color, 0.4);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/playlist/ui/playlist.scss
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
box-sizing: border-box;

.cld-video-player-skin-dark & {
text-shadow: 1px 1px 1px rgba(#000, 0.3);
text-shadow: 0 0 1px var(--color-base);

@media only screen and (max-width: 768px) {
background: var(--color-base);
Expand Down
6 changes: 2 additions & 4 deletions src/plugins/playlist/ui/thumbnail/thumbnail.scss
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,13 @@
bottom: 0;
left: 0;
background: linear-gradient(to top, var(--color-base), transparent 80%);
opacity: 0.6;
opacity: 0.9;
}

&.cld-plw-panel-item-active {
border: 1px solid var(--color-accent);
box-sizing: border-box;
.cld-plw-item-info-wrap {
color: var(--color-accent);
}
box-shadow: 0 0 3em -0.5em var(--color-accent) inset;
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/video-player.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,9 @@ export const overrideDefaultVideojsComponents = () => {
const Player = videojs.getComponent('Player');
let children = Player.prototype.options_.children;

// Add TitleBar as default
children.push('titleBar');
if (children.indexOf('titleBar') === -1) {

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" ?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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

Copy link
Contributor

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.

children.push('titleBar');
}

const ControlBar = videojs.getComponent('ControlBar');
if (ControlBar) {
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/src/pom/colorsApiPage.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Page } from '@playwright/test';
import { VideoComponent } from '../../components/videoComponent';
import { BasePage } from './BasePage';
const COLORS_API_PAGE_COLOR_SKIN_VIDEO_SELECTOR = '//*[@id="vjs_video_3_html5_api"]';
const COLORS_API_PAGE_DARK_SKIN_VIDEO_SELECTOR = '//*[@id="vjs_video_627_html5_api"]';
const COLORS_API_PAGE_LIGHT_SKIN_VIDEO_SELECTOR = '//*[@id="vjs_video_1229_html5_api"]';
const COLORS_API_PAGE_COLOR_SKIN_VIDEO_SELECTOR = '//*[@id="player-1_html5_api"]';
const COLORS_API_PAGE_DARK_SKIN_VIDEO_SELECTOR = '//*[@id="player-2_html5_api"]';
const COLORS_API_PAGE_LIGHT_SKIN_VIDEO_SELECTOR = '//*[@id="player-3_html5_api"]';

/**
* Video player examples colors API page object
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/src/pom/multiplePlayersPage.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Page } from '@playwright/test';
import { VideoComponent } from '../../components/videoComponent';
import { BasePage } from './BasePage';
const MULTIPLE_PLAYERS_PAGE_PLAYER_1_VIDEO_SELECTOR = '//*[@id="vjs_video_3_html5_api"]';
const MULTIPLE_PLAYERS_PAGE_PLAYER_2_VIDEO_SELECTOR = '//*[@id="vjs_video_627_html5_api"]';
const MULTIPLE_PLAYERS_PAGE_PLAYER_3_VIDEO_SELECTOR = '//*[@id="vjs_video_1229_html5_api"]';
const MULTIPLE_PLAYERS_PAGE_PLAYER_1_VIDEO_SELECTOR = '//*[@id="player-1_html5_api"]';
const MULTIPLE_PLAYERS_PAGE_PLAYER_2_VIDEO_SELECTOR = '//*[@id="player-2_html5_api"]';
const MULTIPLE_PLAYERS_PAGE_PLAYER_3_VIDEO_SELECTOR = '//*[@id="player-3_html5_api"]';

/**
* Video player examples colors API page object
Expand Down