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

Add new advertising UI layout #660

Merged
merged 10 commits into from
Dec 20, 2024

Conversation

stonko1994
Copy link
Collaborator

Description

This PR adds the new UI layout for Ad playback according to the current design. The UI looks the same for small screen and default.

Screenshot 2024-12-18 at 13 57 22

Checklist (for PR submitter and reviewers)

  • CHANGELOG entry A general entry will be added at the end

}

.#{$prefix}-ui-button-ad-skip {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The styling for the ad-skip-button was extracted into a separate file.

Comment on lines +572 to +577
// Reset the currentTimeSeekBar and set the position to 0 if the player has no duration
if (this.player.getDuration() === 0) {
this.setPlaybackPosition(0);
currentTimeSeekBar = 0;
return;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was no problem before as there was no seekbar visible during ad playback.

Copy link
Contributor

@bitmovin-kenny bitmovin-kenny left a comment

Choose a reason for hiding this comment

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

Looks nice 💪
But there is a one pixel black border on the left side of the player during ad playback, on a white background even I noticed it:
image


.#{$prefix}-label {
display: inline-block;
color: $color-ads;
Copy link
Contributor

Choose a reason for hiding this comment

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

TBH i find the yellow text hard to read, are we sure we want to stick with that yellow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noted. I'll take that feedback back to the design process.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't your eyes hurt when looking at the above screenshot and trying to figure out how may seconds there are left until skip?

@stonko1994
Copy link
Collaborator Author

stonko1994 commented Dec 20, 2024

Looks nice 💪 But there is a one pixel black border on the left side of the player during ad playback, on a white background even I noticed it: ...

That bothers me since 6 1/2 years 😄 Is also there on development so nothing new. I'll note that and maybe revisit this if time allows.

Screenshot 2024-12-20 at 09 12 13

Copy link
Contributor

@bitmovin-kenny bitmovin-kenny left a comment

Choose a reason for hiding this comment

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

Damn, it's really in dev too

@stonko1994 stonko1994 merged commit 291c093 into feature/modern-ui-base Dec 20, 2024
3 checks passed
@stonko1994 stonko1994 deleted the feature/new-advertising-ui branch December 20, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants