Skip to content

Commit

Permalink
Merge pull request #661 from bitmovin/feature/PW-16297/prevent-seek-t…
Browse files Browse the repository at this point in the history
…humbnails-overflow

Fix seek preview thumbnails exceeding UI dimensions
  • Loading branch information
Andr3wid authored Dec 20, 2024
2 parents c4ab4b3 + 6257705 commit ec13139
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 38 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Fixed
- Seek preview thumbnails exceeding the UI dimensions when default size is increased

## [3.75.0] - 2024-11-19

### Fixed
Expand Down
8 changes: 4 additions & 4 deletions spec/components/seekbar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe('SeekBar', () => {

jest.spyOn(playerMock, 'getSeekableRange').mockImplementation(() => ({start: 26, end: 30}));

seekbar['onSeekPreviewEvent'](40, true)
seekbar['onSeekPreviewEvent'](40, 100, true);

playerMock.eventEmitter.fireSegmentRequestFinished();

Expand All @@ -127,19 +127,19 @@ describe('SeekBar', () => {
it('will update the scrubber location after a successful segment request download and the user is not scrubbing', () => {
jest.spyOn(playerMock, 'getSeekableRange').mockImplementation(() => ({start: 26, end: 30}));

seekbar['onSeekPreviewEvent'](18, false)
seekbar['onSeekPreviewEvent'](18, 100, false);

playerMock.eventEmitter.fireSegmentRequestFinished();

expect(setPlaybackPositionSpy).toHaveBeenLastCalledWith(18);
expect(setBufferPositionSpy).toHaveBeenLastCalledWith(18)
expect(setBufferPositionSpy).toHaveBeenLastCalledWith(18);
});
});
});

describe('group playback', () => {
beforeEach(() => {
jest.spyOn(playerMock, 'getDuration').mockReturnValue(0)
jest.spyOn(playerMock, 'getDuration').mockReturnValue(0);
seekbar.configure(playerMock, uiInstanceManagerMock);
});

Expand Down
94 changes: 94 additions & 0 deletions spec/components/seekbarlabel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { MockHelper, TestingPlayerAPI } from '../helper/MockHelper';
import { UIInstanceManager } from '../../src/ts/uimanager';
import { SeekBarLabel } from '../../src/ts/components/seekbarlabel';
import { SeekPreviewEventArgs } from '../../src/ts/components/seekbar';
import { DOM } from '../../src/ts/dom';

let playerMock: TestingPlayerAPI;
let uiInstanceManagerMock: UIInstanceManager;
Expand Down Expand Up @@ -94,4 +95,97 @@ describe('SeekBarLabel', () => {
});
});
});

describe("calculates correct values for thumbnail positioning", () => {
const uiContainerBoundingRect = {
x: 200,
y: 150,
width: 1600,
height: 900,
top: 150,
right: 1800,
bottom: 1050,
left: 200,
} as DOMRect;

let containerGetDomElementMock: () => jest.Mocked<DOM>;
let caretGetDomElementMock: () => jest.Mocked<DOM>;

beforeEach(() => {
containerGetDomElementMock = jest
.fn()
.mockReturnValue(MockHelper.generateDOMMock());

containerGetDomElementMock().get = jest.fn().mockReturnValue({
parentElement: jest.fn().mockReturnValue({
getBoundingClientRect: jest.fn(),
}),
});

caretGetDomElementMock = jest.fn().mockReturnValue(MockHelper.generateDOMMock());

seekbarLabel["container"].getDomElement = containerGetDomElementMock;
seekbarLabel["caret"].getDomElement = caretGetDomElementMock;
});

it("when thumbnail within UI container bounds", () => {
const labelRect = {
x: 400,
y: 700,
width: 200,
height: 120,
top: 700,
right: 600,
bottom: 820,
left: 400,
} as DOMRect;

containerGetDomElementMock().get(0).parentElement!.getBoundingClientRect =
jest.fn().mockReturnValue(labelRect);

seekbarLabel.setPositionInBounds(100, uiContainerBoundingRect);

expect(caretGetDomElementMock().css).toHaveBeenCalledWith('transform', null);
});

it("when thumbnail would overflow UI container leftside", () => {
const labelRect = {
x: 180,
y: 700,
width: 200,
height: 120,
top: 700,
right: 380,
bottom: 820,
left: 180,
} as DOMRect;

containerGetDomElementMock().get(0).parentElement!.getBoundingClientRect =
jest.fn().mockReturnValue(labelRect);

seekbarLabel.setPositionInBounds(100, uiContainerBoundingRect);

expect(seekbarLabel.getDomElement().css).toHaveBeenCalledWith('left', '120px');
});

it("when thumbnail would overflow UI container rightside", () => {
const labelRect = {
x: 1650,
y: 700,
width: 200,
height: 120,
top: 700,
right: 1850,
bottom: 820,
left: 1650,
} as DOMRect;

containerGetDomElementMock().get(0).parentElement!.getBoundingClientRect =
jest.fn().mockReturnValue(labelRect);

seekbarLabel.setPositionInBounds(100, uiContainerBoundingRect);

expect(seekbarLabel.getDomElement().css).toHaveBeenCalledWith('left', '50px');
});
});
});
33 changes: 17 additions & 16 deletions src/scss/skin-modern/components/_seekbarlabel.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,28 @@

> .#{$prefix}-container-wrapper {
@extend %center-on-left-edge;

padding-left: 1em;
padding-right: 1em;
}

// bottom arrow from http://www.cssarrowplease.com/
.#{$prefix}-seekbar-label-caret {
border: solid transparent;
border-color: transparent;
border-top-color: $color-primary;
border-width: .5em;
height: 0;
margin-left: -.5em;
pointer-events: none;
position: absolute;
top: 100%;
width: 0;
}

.#{$prefix}-seekbar-label-inner {
border-bottom: .2em solid $color-primary;

// bottom arrow from http://www.cssarrowplease.com/
&::after {
border: solid transparent;
border-color: transparent;
border-top-color: $color-primary;
border-width: .5em;
content: ' ';
height: 0;
left: 50%;
margin-left: -.5em;
pointer-events: none;
position: absolute;
top: 100%;
width: 0;
}

> .#{$prefix}-container-wrapper {
position: relative;

Expand Down
34 changes: 25 additions & 9 deletions src/ts/components/seekbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ export class SeekBar extends Component<SeekBarConfig> {
private seekBarMarkersContainer: DOM;
private timelineMarkersHandler: TimelineMarkersHandler;

private uiBoundingRect: DOMRect;

private player: PlayerAPI;
private uiManager: UIInstanceManager;

protected seekBarType: SeekBarType;

Expand Down Expand Up @@ -220,6 +223,7 @@ export class SeekBar extends Component<SeekBarConfig> {
super.configure(player, uimanager);

this.player = player;
this.uiManager = uimanager;

// Apply scaling transform to the backdrop bar to have all bars rendered similarly
// (the call must be up here to be executed for the volume slider as well)
Expand Down Expand Up @@ -453,6 +457,7 @@ export class SeekBar extends Component<SeekBarConfig> {
// is positioned absolutely and must therefore be updated when the size of the seekbar changes.
player.on(player.exports.PlayerEvent.PlayerResized, () => {
this.refreshPlaybackPosition();
this.uiBoundingRect = this.uiManager.getUI().getDomElement().get(0).getBoundingClientRect();
});
// Additionally, when this code is called, the seekbar is not part of the UI yet and therefore does not have a size,
// resulting in a wrong initial position of the marker. Refreshing it once the UI is configured solved this issue.
Expand Down Expand Up @@ -716,10 +721,13 @@ export class SeekBar extends Component<SeekBarConfig> {
e.stopPropagation();
}

let targetPercentage = 100 * this.getOffset(e);
const offset = this.getOffset(e);
const targetPercentage = 100 * offset;
const seekPositionPx = offset * this.seekBar.width();

this.setSeekPosition(targetPercentage);
this.setPlaybackPosition(targetPercentage);
this.onSeekPreviewEvent(targetPercentage, true);
this.onSeekPreviewEvent(targetPercentage, seekPositionPx, true);
};

let mouseTouchUpHandler = (e: MouseEvent | TouchEvent) => {
Expand Down Expand Up @@ -777,10 +785,12 @@ export class SeekBar extends Component<SeekBarConfig> {
mouseTouchMoveHandler(e);
}

let position = 100 * this.getOffset(e);
this.setSeekPosition(position);
const offset = this.getOffset(e);
const seekPositionPercentage = 100 * offset;
const seekPositionPx = offset * this.seekBar.width();

this.onSeekPreviewEvent(position, false);
this.setSeekPosition(seekPositionPercentage);
this.onSeekPreviewEvent(seekPositionPercentage, seekPositionPx, false);

if (this.hasLabel() && this.getLabel().isHidden()) {
this.getLabel().show();
Expand Down Expand Up @@ -1020,7 +1030,15 @@ export class SeekBar extends Component<SeekBarConfig> {
this.seekBarEvents.onSeek.dispatch(this);
}

protected onSeekPreviewEvent(percentage: number, scrubbing: boolean) {
private updateLabelPosition = (pixelPosition: number) => {
if (!this.uiBoundingRect) {
this.uiBoundingRect = this.uiManager.getUI().getDomElement().get(0).getBoundingClientRect();
}

this.label.setPositionInBounds(pixelPosition, this.uiBoundingRect);
};

protected onSeekPreviewEvent(percentage: number, targetOffsetPx: number, scrubbing: boolean) {
let snappedMarker = this.timelineMarkersHandler && this.timelineMarkersHandler.getMarkerAtPosition(percentage);

let seekPositionPercentage = percentage;
Expand All @@ -1043,9 +1061,7 @@ export class SeekBar extends Component<SeekBarConfig> {
}

if (this.label) {
this.label.getDomElement().css({
'left': seekPositionPercentage + '%',
});
this.updateLabelPosition(targetOffsetPx);
}

this.seekBarEvents.onSeekPreview.dispatch(this, {
Expand Down
47 changes: 38 additions & 9 deletions src/ts/components/seekbarlabel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export class SeekBarLabel extends Container<SeekBarLabelConfig> {
private appliedMarkerCssClasses: string[] = [];
private player: PlayerAPI;
private uiManager: UIInstanceManager;
private readonly container: Container<ContainerConfig>;
private readonly caret: Label<LabelConfig>;

constructor(config: SeekBarLabelConfig = {}) {
super(config);
Expand All @@ -45,17 +47,22 @@ export class SeekBarLabel extends Container<SeekBarLabelConfig> {
this.thumbnail = new Component({ cssClasses: ['seekbar-thumbnail'], role: 'img' });
this.thumbnailImageLoader = new ImageLoader();

this.container = new Container({
components: [
this.thumbnail,
new Container({
components: [this.titleLabel, this.timeLabel],
cssClass: 'seekbar-label-metadata',
}),
],
cssClass: 'seekbar-label-inner',
});

this.caret = new Label({ cssClasses: ['seekbar-label-caret'] });

this.config = this.mergeConfig(config, {
cssClass: 'ui-seekbar-label',
components: [new Container({
components: [
this.thumbnail,
new Container({
components: [this.titleLabel, this.timeLabel],
cssClass: 'seekbar-label-metadata',
})],
cssClass: 'seekbar-label-inner',
})],
components: [this.container, this.caret],
hidden: true,
}, this.config);
}
Expand Down Expand Up @@ -129,6 +136,28 @@ export class SeekBarLabel extends Container<SeekBarLabelConfig> {
}
};

public setPositionInBounds(seekPositionPx: number, bounds: DOMRect) {
this.getDomElement().css('left', seekPositionPx + 'px');

// Check parent container as it has a padding that needs to be considered
const labelBounding = this.container.getDomElement().get(0).parentElement.getBoundingClientRect();

let preventOverflowOffset = 0;
if (labelBounding.right > bounds.right) {
preventOverflowOffset = labelBounding.right - bounds.right;
} else if (labelBounding.left < bounds.left) {
preventOverflowOffset = labelBounding.left - bounds.left;
}

if (preventOverflowOffset !== 0) {
this.getDomElement().css('left', seekPositionPx - preventOverflowOffset + 'px');

this.caret.getDomElement().css('transform', `translateX(${preventOverflowOffset}px)`);
} else {
this.caret.getDomElement().css('transform', null);
}
}

/**
* Sets arbitrary text on the label.
* @param text the text to show on the label
Expand Down

0 comments on commit ec13139

Please sign in to comment.