Skip to content

Commit

Permalink
fix(inlineplayer): inline layout UI and a11y fixes
Browse files Browse the repository at this point in the history
* fix(inlineplayer): not visible on ios 14 and lower

* fix(inlineplayer): paywall overflowing on ios 14

* chore: improved fix for aspect ratio player on ios

* chore: fix design feedback

* chore: use backgroud image instead of shelfimage
for paywall

* fix: logo stretched under some conditions

* fix(project): lighthouse accessibility issues

* chore: tweaked filter labels

* chore: fix end2end test

* chore: fix failing e2e test playlist

* chore: update translations

* chore: fix tabIndex video list item
  • Loading branch information
RCVZ authored Oct 5, 2022
1 parent 5670fd8 commit 71c9666
Show file tree
Hide file tree
Showing 25 changed files with 80 additions and 92 deletions.
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
"jwt-decode": "^3.1.2",
"lodash.merge": "^4.6.2",
"marked": "^4.1.1",
"memoize-one": "^5.2.1",
"npm-run-all": "^4.1.5",
"planby": "^0.3.0",
"react": "^17.0.2",
Expand All @@ -58,7 +57,6 @@
"react-infinite-scroller": "^1.2.6",
"react-query": "^3.13.10",
"react-router-dom": "^6.4.0",
"react-virtualized": "^9.22.3",
"wicg-inert": "^3.1.1",
"yup": "^0.32.9",
"zustand": "^3.6.9"
Expand Down
1 change: 1 addition & 0 deletions src/components/Button/Button.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ $large-button-height: 40px;
&:not(.disabled) {
&:hover,
&:focus {
z-index: 1;
transform: scale(1.1);
}

Expand Down
5 changes: 2 additions & 3 deletions src/components/Card/Card.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
transform: scale(1.05);

& .poster {
box-shadow: 0 0 0 3px var(--highlight-color, variables.$white), 0 8px 10px rgb(0 0 0 / 14%), 0 3px 14px rgb(0 0 0 / 12%),
0 4px 5px rgb(0 0 0 / 20%);
box-shadow: 0 0 0 3px var(--highlight-color, variables.$white), 0 8px 10px rgb(0 0 0 / 14%), 0 3px 14px rgb(0 0 0 / 12%), 0 4px 5px rgb(0 0 0 / 20%);
}
}
}
Expand Down Expand Up @@ -202,7 +201,7 @@ $aspects: ((1, 1), (2, 1), (2, 3), (4, 3), (5, 3), (16, 9), (9, 16), (9, 13));
padding: 2px 6px;

> svg {
width: 16px;
width: 14px;
height: 21px;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Card/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ function Card({
{featured && !disabled && enableTitle && <div className={classNames(styles.title, { [styles.loading]: loading })}>{title}</div>}
<div className={styles.tags}>
{isLocked && (
<div className={classNames(styles.tag, styles.lock)} aria-label={t('card_lock')}>
<div className={classNames(styles.tag, styles.lock)} aria-label={t('card_lock')} role="status">
<Lock />
</div>
)}
Expand Down
4 changes: 3 additions & 1 deletion src/components/CardGrid/CardGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ function CardGrid({
hasMore={rowCount * LOAD_ROWS_COUNT < playlist.playlist.length - 1}
loader={<InfiniteScrollLoader key="loader" />}
>
<div className={classNames(styles.container, styles[`cols-${visibleTiles}`])}>{playlist.playlist.slice(0, rowCount * visibleTiles).map(renderTile)}</div>
<div className={classNames(styles.container, styles[`cols-${visibleTiles}`])} role="grid">
{playlist.playlist.slice(0, rowCount * visibleTiles).map(renderTile)}
</div>
</InfiniteScroll>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ exports[`<CardGrid> > renders and matches snapshot 1`] = `
<div>
<div
class="_container_7175d1 _cols-5_7175d1"
role="grid"
>
<div
class="_cell_7175d1"
Expand Down
4 changes: 2 additions & 2 deletions src/components/Filter/Filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const Filter: FC<Props> = ({ name, value, defaultLabel, options, setValue, value
return (
<Fragment>
{showFilterRow ? (
<div className={styles.filterRow} role="listbox">
<div className={styles.filterRow} role="listbox" aria-label={t('filter_videos_by', { name })}>
{options.map((option) => (
<Button label={`${valuePrefix}${option}`} onClick={() => setValue(option)} key={option} active={value === option} role="option" />
))}
Expand All @@ -49,7 +49,7 @@ const Filter: FC<Props> = ({ name, value, defaultLabel, options, setValue, value
name={name}
value={value}
onChange={handleChange}
aria-label={t('filter_videos_by_genre')}
aria-label={t('filter_videos_by', { name })}
/>
</div>
)}
Expand Down
1 change: 1 addition & 0 deletions src/components/Filter/__snapshots__/Filter.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`<Filter> > renders Filter 1`] = `
<div>
<div
aria-label="filter_videos_by"
class="_filterRow_2153bc"
role="listbox"
>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Logo/Logo.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// --------------------------------

.logo {
max-width: 100%;
width: auto;
max-height: variables.$header-height - 10px;
vertical-align: middle;
cursor: pointer;
Expand Down
18 changes: 9 additions & 9 deletions src/components/UserMenu/UserMenu.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@
letter-spacing: 0.15px;
background: theme.$modal-bg;
box-shadow: theme.$panel-box-shadow;

> ul {
margin: 0;
padding: 0;
list-style-type: none;
}
}

.menuItems > li > a {
padding: 0 18px;
font-size: 16px;
.menuItems {
margin: 0;
padding: 0;
list-style-type: none;

> li > a {
padding: 0 18px;
font-size: 16px;
}
}

.button {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

@include responsive.mobile-and-small-tablet() {
flex-wrap: wrap;

border-bottom: 0;
> button:nth-last-child(3) {
flex: 1 1 100%;
margin: 0 0 8px 0;
Expand Down
18 changes: 18 additions & 0 deletions src/components/VideoLayout/VideoLayout.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,23 @@
}

.player {
position: relative;
grid-area: player;
aspect-ratio: 16 / 9;
box-shadow: 0 8px 10px rgb(0 0 0 / 14%), 0 3px 14px rgb(0 0 0 / 12%), 0 4px 5px rgb(0 0 0 / 20%);

@supports not (aspect-ratio: 16 / 9) {
&::after {
display: block;
content: '';
clear: both;
}
&::before {
float: left;
padding-top: 56.25%;
content: '';
}
}

@include responsive.mobile-only() {
margin: -16px -16px 0;
Expand Down Expand Up @@ -87,6 +103,7 @@
align-items: initial;
width: 100%;
grid-area: related;
padding-bottom: 0;

> h3 {
font-size: 20px;
Expand Down Expand Up @@ -123,6 +140,7 @@
}

.filtersInline {
margin-bottom: variables.$base-spacing;
padding-bottom: variables.$base-spacing;
border-bottom: 1px solid rgba(255, 255, 255, 0.3);
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/VideoLayout/VideoLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ const VideoLayout: React.FC<Props> = ({
{!!filterMetadata && inlineLayout && <span className={styles.filterMetadata}>{filterMetadata}</span>}
{showFilters && (
<Filter
name="categories"
name="season"
value={currentFilter}
valuePrefix={filterValuePrefix}
defaultLabel={defaultFilterLabel}
Expand Down
11 changes: 4 additions & 7 deletions src/components/VideoListItem/VideoListItem.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -95,24 +95,21 @@
right: 8px;
bottom: 8px;
display: flex;

@include responsive.mobile-only {
font-size: 14px;
}
}

svg.lock {
width: 25px;
height: 25px;
width: 24px;
height: 24px;
padding: 5px 5px;
color: var(--card-color);
background-color: rgba(variables.$black, 0.6);
border-radius: 4px;
}

.tag {
margin-left: 8px;
margin-left: 4px;
padding: 2px 6px;
font-size: 14px;
}

.live {
Expand Down
10 changes: 9 additions & 1 deletion src/components/VideoListItem/VideoListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,15 @@ function VideoListItem({
const handleKeyDown = (event: KeyboardEvent) => (event.key === 'Enter' || event.key === ' ') && onClick && onClick();

return (
<div className={styles.listItem} onClick={onClick} onMouseEnter={onHover} onKeyDown={handleKeyDown} role="button" aria-label={t('play_item', { title })}>
<div
className={styles.listItem}
onClick={onClick}
onMouseEnter={onHover}
onKeyDown={handleKeyDown}
role="button"
aria-label={t('play_item', { title })}
tabIndex={0}
>
<div className={styles.poster}>
<Image className={posterImageClassNames} image={image} alt={title} onLoad={() => setImageLoaded(true)} width={320} />
{isActive && <div className={styles.activeLabel}>{activeLabel}</div>}
Expand Down
28 changes: 8 additions & 20 deletions src/containers/InlinePlayer/InlinePlayer.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@
@use '../../styles/mixins/typography';
@use '../../styles/mixins/responsive';

.inlinePlayer {
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
}

.paywall {
position: absolute;
top: 0;
Expand Down Expand Up @@ -83,23 +91,3 @@
line-height: 1em;
}
}

.inlinePlayer {
position: relative;
aspect-ratio: 16 / 9;
box-shadow: 0 8px 10px rgb(0 0 0 / 14%), 0 3px 14px rgb(0 0 0 / 12%), 0 4px 5px rgb(0 0 0 / 20%);

@supports not (aspect-ratio: 16 / 9) {
&::before {
float: left;
padding-top: 56.25;
content: '';
}

&::after {
display: block;
content: '';
clear: both;
}
}
}
2 changes: 1 addition & 1 deletion src/containers/InlinePlayer/InlinePlayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const InlinePlayer: React.FC<Props> = ({
<div className={styles.inlinePlayer}>
<Fade open={paywall}>
<div className={styles.paywall}>
<Image className={styles.poster} image={item.shelfImage} alt={item.title} width={1280} />
<Image className={styles.poster} image={item.backgroundImage} alt={item.title} width={1280} />
<Lock className={styles.lock} />
<h2 className={styles.title}>{t('video:sign_up_to_start_watching')}</h2>
<span className={styles.text}>{t('account:choose_offer.watch_this_on_platform', { siteName })}</span>
Expand Down
1 change: 1 addition & 0 deletions src/containers/ShelfList/ShelfList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const ShelfList = ({ rows }: Props) => {
style={{ overflow: 'hidden' }}
loadMore={() => setRowCount((current) => current + LOAD_ROWS_COUNT)}
hasMore={rowCount < rows.length - 1}
role="grid"
loader={<InfiniteScrollLoader key="loader" />}
>
{rows.slice(0, rowCount).map((row, index) => (
Expand Down
6 changes: 4 additions & 2 deletions src/i18n/locales/en_US/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@
"confirm": "Yes"
},
"default_site_name": "My OTT Application",
"filter_videos_by_genre": "Filter videos by genre",
"filter_videos_by": "Filter videos by {{ name }}",
"home": "Home",
"live": "LIVE",
"now": "Now",
"optional": "(Optional)",
"play_item": "Play {{ title }}",
"series": "series",
"sign_in": "Sign in",
"sign_up": "Sign up",
"slide_left": "Slide left",
"slide_right": "Slide right"
"slide_right": "Slide right",
"filter_videos_by_genre": "Filter videos by genre"
}
2 changes: 2 additions & 0 deletions src/i18n/locales/en_US/video.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
"copied_url": "Copied url",
"current_episode": "Current episode",
"current_video": "Current video",
"episode": "episode",
"episode_not_found": "Episode not found",
"episodes": "Episodes",
"favorite": "Favorite",
"favorites_warning": "Maximum amount of favorite videos exceeded. You can only add up to {{maxCount}} favorite videos. Please delete one and repeat the operation.",
"remove_from_favorites": "Remove from favorites",
"season": "season",
"season_prefix": "Season ",
"series_error": "An error occurred while requesting series",
"share": "Share",
Expand Down
6 changes: 4 additions & 2 deletions src/i18n/locales/nl_NL/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@
"confirm": ""
},
"default_site_name": "",
"filter_videos_by_genre": "",
"filter_videos_by": "",
"home": "",
"live": "",
"now": "",
"optional": "",
"play_item": "",
"series": "",
"sign_in": "",
"sign_up": "",
"slide_left": "",
"slide_right": ""
"slide_right": "",
"filter_videos_by_genre": ""
}
3 changes: 3 additions & 0 deletions src/i18n/locales/nl_NL/video.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
"continue_watching": "",
"copied_url": "",
"current_episode": "",
"current_video": "",
"episode": "",
"episode_not_found": "",
"episodes": "",
"favorite": "",
"favorites_warning": "",
"remove_from_favorites": "",
"season": "",
"season_prefix": "",
"series_error": "",
"share": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const PlaylistGrid: ScreenComponent<Playlist> = ({ data, isLoading }) => {
</Helmet>
<header className={styles.header}>
<h2>{data.title}</h2>
{shouldShowFilter && <Filter name="categories" value={filter} defaultLabel="All" options={categories} setValue={setFilter} />}
{shouldShowFilter && <Filter name="genre" value={filter} defaultLabel="All" options={categories} setValue={setFilter} />}
</header>
<main className={styles.main}>
<CardGrid
Expand Down
2 changes: 1 addition & 1 deletion test-e2e/tests/inline_layout_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Scenario('I switch to another season in the video list', async ({ I }) => {
I.see('S1:E1 - Welcome');

I.see('Season 1/4 - Episode 1/6');
I.selectOption({ css: 'select[name="categories"]' }, 'Season 2');
I.selectOption({ css: 'select[name="season"]' }, 'Season 2');

I.click(locate({ css: 'div[aria-label="Play Choosing a skin (Cycles Render)"]' }).inside(videoListLocator));
I.dontSee('Season 1/4 - Episode 1/6');
Expand Down
Loading

0 comments on commit 71c9666

Please sign in to comment.