Skip to content

Commit 5d5eaf8

Browse files
HenriqueLimasclaude
HenriqueLimas
and
claude
authored
feat(video): align React component with Marko and shaka-player v5 (#570)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0605956 commit 5d5eaf8

28 files changed

+828
-336
lines changed

‎.changeset/video-portal-play-button.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@ebay/ui-core-react": patch
3+
"@ebay/ebayui-core": patch
4+
"@ebay/skin": patch
5+
---
6+
7+
**@ebay/ui-core-react:** Align video component with Marko implementation and shaka-player v5 - refactor to declarative approach with createPortal, implement missing controls, fix icon re-rendering and autoplay behavior.
8+
9+
**@ebay/ebayui-core:** Add accessible button wrapper for play button.
10+
11+
**@ebay/skin:** Add button reset styles for shaka-play-button.

‎packages/ebayui-core-react/src/ebay-icon/context.tsx‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
1-
import React, { createContext, type ReactNode } from "react";
1+
import React, { createContext, type ReactNode, useRef } from "react";
22

33
export const IconContext = createContext<Set<string>>(null);
44

55
export const ROOT_ID = "ebayui-svg-symbols";
6-
export const EbayIconProvider = ({ children }: { children: ReactNode }) => (
7-
<IconContext.Provider value={new Set()}>
8-
<svg
9-
id={ROOT_ID}
10-
style={{ position: "absolute", height: "0px", width: "0px" }}
11-
focusable={false}
12-
aria-hidden="true"
13-
/>
14-
{children}
15-
</IconContext.Provider>
16-
);
6+
export const EbayIconProvider = ({ children }: { children: ReactNode }) => {
7+
// Use ref to maintain the same Set instance across re-renders
8+
const lookupRef = useRef<Set<string>>(new Set());
9+
10+
return (
11+
<IconContext.Provider value={lookupRef.current}>
12+
<svg
13+
id={ROOT_ID}
14+
style={{ position: "absolute", height: "0px", width: "0px" }}
15+
focusable={false}
16+
aria-hidden="true"
17+
/>
18+
{children}
19+
</IconContext.Provider>
20+
);
21+
};

‎packages/ebayui-core-react/src/ebay-icon/icon.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,14 @@ const EbayIcon: FC<EbayIconProps> = ({
136136
};
137137
const kebabName = kebabCased(name);
138138
const size = getIconSize(kebabName) || kebabName;
139+
const isColored = kebabName.endsWith("-colored");
139140

140141
const classNamePrefix = __type === "flag" ? "flag" : "icon";
141-
const skinClassName = [`${classNamePrefix}`, `${classNamePrefix}--${size}`, getFilledIconName(kebabName)]
142+
const skinClassName = [
143+
`${classNamePrefix}`,
144+
`${classNamePrefix}--${size}${isColored ? "-colored" : ""}`,
145+
getFilledIconName(kebabName),
146+
]
142147
.filter(Boolean)
143148
.join(" ");
144149
const className = classNames(extraClass, {

‎packages/ebayui-core-react/src/ebay-video/__tests__/index.spec.tsx‎

Lines changed: 85 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
11
import React from "react";
2-
import { render, screen } from "@testing-library/react";
2+
import { render, screen, waitFor } from "@testing-library/react";
3+
import userEvent from "@testing-library/user-event";
34
import { EbayVideo, EbayVideoSource } from "../index";
45

56
describe("<EbayVideo>", () => {
7+
const defaultProps = {
8+
thumbnail: "https://ir.ebaystatic.com/cr/v/c1/ebayui/video/v1/iphone-thumbnail.jpg",
9+
width: 500,
10+
height: 300,
11+
a11yPlayText: "Play",
12+
errorText: "Error loading",
13+
reportText: "Report",
14+
};
15+
616
beforeEach(() =>
717
render(
8-
<EbayVideo
9-
thumbnail="https://ir.ebaystatic.com/cr/v/c1/ebayui/video/v1/iphone-thumbnail.jpg"
10-
width={500}
11-
height={300}
12-
a11yPlayText="Play"
13-
a11yLoadText="Loading"
14-
errorText="Error loading"
15-
reportText="Report"
16-
>
18+
<EbayVideo {...defaultProps}>
1719
<EbayVideoSource src="https://ir.ebaystatic.com/cr/v/c1/ebayui/video/v1/video.mp4" />
1820
</EbayVideo>,
1921
),
@@ -30,10 +32,81 @@ describe("<EbayVideo>", () => {
3032
});
3133

3234
it("shows play button", () => {
33-
expect(screen.getByLabelText("Play")).toBeInTheDocument();
35+
const playButtons = screen.getAllByLabelText("Play");
36+
expect(playButtons.length).toBeGreaterThan(0);
3437
});
3538

3639
it("shows loading spinner", () => {
37-
expect(screen.getByLabelText("Loading")).toBeInTheDocument();
40+
const spinner = document.querySelector(".shaka-spinner");
41+
expect(spinner).toBeInTheDocument();
42+
});
43+
44+
it("renders initial play button in shaka controls container", () => {
45+
const playButtonContainer = document.querySelector(".shaka-play-button-container");
46+
expect(playButtonContainer).toBeInTheDocument();
47+
48+
const shakaControls = document.querySelector(".shaka-controls-container");
49+
expect(shakaControls).toBeInTheDocument();
50+
51+
// Verify play button is inside shaka controls
52+
expect(shakaControls?.querySelector(".shaka-play-button")).toBeInTheDocument();
53+
});
54+
55+
it("play button has correct icon with width 64", () => {
56+
const playButtonContainer = document.querySelector(".shaka-play-button-container");
57+
const playButton = playButtonContainer?.querySelector(".shaka-play-button");
58+
expect(playButton).toBeInTheDocument();
59+
60+
const icon = playButton?.querySelector("svg");
61+
expect(icon).toHaveAttribute("width", "64");
62+
expect(icon).toHaveClass("icon--64-colored");
63+
});
64+
65+
it("removes initial play button on click", async () => {
66+
const user = userEvent.setup();
67+
const playButtonContainer = document.querySelector(".shaka-play-button-container");
68+
const playButton = playButtonContainer?.querySelector(".shaka-play-button");
69+
expect(playButton).toBeInTheDocument();
70+
71+
await user.click(playButton!);
72+
73+
await waitFor(() => {
74+
const playButtonContainerAfterClick = document.querySelector(".shaka-play-button-container");
75+
// The initial play button should be removed from React tree
76+
expect(playButtonContainerAfterClick).not.toBeInTheDocument();
77+
});
78+
});
79+
80+
describe("autoplay behavior", () => {
81+
it("sets autoplay attribute when autoplay prop is true", () => {
82+
const { container } = render(
83+
<EbayVideo {...defaultProps} autoPlay>
84+
<EbayVideoSource src="https://ir.ebaystatic.com/cr/v/c1/ebayui/video/v1/video.mp4" />
85+
</EbayVideo>,
86+
);
87+
88+
const video = container.querySelector("video") as HTMLVideoElement;
89+
expect(video.autoplay).toBe(true);
90+
});
91+
92+
it("triggers play when action prop changes to 'play'", () => {
93+
const { container, rerender } = render(
94+
<EbayVideo {...defaultProps}>
95+
<EbayVideoSource src="https://ir.ebaystatic.com/cr/v/c1/ebayui/video/v1/video.mp4" />
96+
</EbayVideo>,
97+
);
98+
99+
const video = container.querySelector("video") as HTMLVideoElement;
100+
const playSpy = vi.spyOn(video, "play").mockImplementation(() => Promise.resolve());
101+
102+
// Change action to 'play'
103+
rerender(
104+
<EbayVideo {...defaultProps} action="play">
105+
<EbayVideoSource src="https://ir.ebaystatic.com/cr/v/c1/ebayui/video/v1/video.mp4" />
106+
</EbayVideo>,
107+
);
108+
109+
expect(playSpy).toHaveBeenCalled();
110+
});
38111
});
39112
});

‎packages/ebayui-core-react/src/ebay-video/__tests__/index.stories.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,9 @@ import "shaka-player/dist/controls.css";
9797
} as Meta;
9898

9999
const defaultProps: EbayVideoProps = {
100-
a11yLoadText: "Loading",
101-
a11yPlayText: "Click to play",
100+
a11yPlayText: "Play",
102101
errorText: "An error has occurred",
103-
width: 600,
102+
width: 700,
104103
height: 400,
105104
onPlay: (e: SyntheticEvent<HTMLVideoElement>, props: PlayEventProps) => action("onPlay")(e, props),
106105
onVolumeChange: (e: SyntheticEvent<HTMLVideoElement>, props: VolumeChangeProps) =>

‎packages/ebayui-core-react/src/ebay-video/__tests__/render.spec.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ describe("ebay-video rendering", () => {
1010
const { container } = render(<DefaultStory />);
1111
const player = container.querySelector(".video-player");
1212
expect(player).toHaveClass("video-player--poster");
13-
expect(player).toHaveAttribute("style", "width: 600px; height: 400px;");
13+
expect(player).toHaveAttribute("style", "width: 700px; height: 400px;");
1414

1515
const videoContainer = player.querySelector(".video-player__container");
1616
expect(videoContainer).toHaveClass("shaka-video-container");
@@ -52,6 +52,5 @@ describe("ebay-video rendering", () => {
5252
"poster",
5353
"https://ir.ebaystatic.com/cr/v/c1/ebayui/video/v1/iphone-thumbnail.jpg",
5454
);
55-
expect(video).toHaveAttribute("style", "width: 600px; height: 400px;");
5655
});
5756
});

‎packages/ebayui-core-react/src/ebay-video/const.ts‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,3 @@
11
export const ERROR_ANOTHER_LOAD = 7000;
22
export const ERROR_NO_PLAYER = 11;
3-
export const defaultVideoConfig = {
4-
addSeekBar: true,
5-
controlPanelElements: [
6-
"play_pause",
7-
"current_time",
8-
"spacer",
9-
"total_time",
10-
"mute",
11-
"report",
12-
"captions",
13-
// 'quality', // uncomment this to show a gear icon for video quality control
14-
"fullscreen",
15-
],
16-
};
3+
export const DEFAULT_SPINNER_TIMEOUT = 2000;

0 commit comments

Comments
 (0)