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

Fix seek preview thumbnails exceeding UI dimensions #661

Merged
merged 11 commits into from
Dec 20, 2024
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
Loading