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

bug: [UEPR-118] Player sound playback #8980

Conversation

Bogomil-Stoyanov
Copy link
Contributor

@Bogomil-Stoyanov Bogomil-Stoyanov commented Dec 2, 2024

Jira ticket:
UEPR-118

@Bogomil-Stoyanov Bogomil-Stoyanov changed the base branch from develop to integration-branch-ux-12.2024 December 2, 2024 07:28
@@ -75,6 +72,12 @@ class Intro extends React.Component {
</div>
)
}

<Video
className={classnames('intro-videos', !this.state.videoOpen && 'invisible')}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the invisible class if we don't render the video component when this.state.videoOpen is false?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this taken out of the this.state.videoOpen check?

Copy link
Contributor Author

@Bogomil-Stoyanov Bogomil-Stoyanov Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is to ensure that the video component fully loads before it is displayed. The bug occurred because the player was not properly initialized, causing the video to start playing before the player was ready.
I am not exactly sure why it is happening

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the style, it was left behind from another previous solution that I tried

@@ -35,44 +40,53 @@ class Video extends React.Component {
onReady: video => {
video.bind('play', () => {
this.setState({videoStarted: true});
this.props.onVideoStart();
if (this.props.onVideoStart) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering whether it would be better to give onVideoStart a default value. But I don't mind it either way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before that there wasn't even a check, it was just straight up throwing an error in the console 😅

@Bogomil-Stoyanov Bogomil-Stoyanov merged commit b975170 into scratchfoundation:integration-branch-ux-12.2024 Dec 3, 2024
2 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants