Skip to content
This repository was archived by the owner on Apr 18, 2024. It is now read-only.

Commit ac4a98d

Browse files
bmartelbmartel
authored
fix: DEV-2019: Sync video timeline seeker with frame position (#1153)
* fix: DEV-2019: Sync video timeline seeker with frame position * fix the scroll indicator position * adding e2e test for video timeline seeking * removing unused change * removing the addition to offset steps for scroll and position change, as it is incorrectly inflating the transition points of the seeker and indicator * duration should visually represent the total length * fix off by one error on indicator selection * ensure the scroll view returns to the very start * fix test * ensure frameNumber is calculated as ceil to align with all other frame calculations for positional frames * ensure scroll of video frames correctly updates and maintains the correct positional offsets in both large jumps and the singular frame steps * augment the seeker width by a pixel and a half, to combat rounding issues in most cases caused by the percentage of a calculated pixel to be largely off * fix test * store the previous slide scroll in all cases to allow future large jump calcs to be correct * fix the seek indicator drag not updating the timeline scroll correctly * fix the seek indicator drag update for offsetX when the lastScrollPosition was far out of view * updating test, adding comments * updating test, avoiding the rounding errors
1 parent 63f7e01 commit ac4a98d

File tree

9 files changed

+308
-39
lines changed

9 files changed

+308
-39
lines changed

‎e2e/fragments/AtVideoView.js‎

Lines changed: 87 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,89 @@
1-
/* global locate */
2-
3-
module.exports = {
1+
/* global inject */
2+
/* global locate */
3+
const { I } = inject();
4+
5+
/**
6+
* @typedef BoundingClientRect
7+
* @property {number} x
8+
* @property {number} y
9+
* @property {number} width
10+
* @property {number} height
11+
*/
12+
13+
module.exports = {
414
_rootSelector: '.lsf-video-segmentation',
515
_videoRootSelector: '.lsf-video__main',
6-
locateVideoContainer() {
7-
return locate(this._videoRootSelector);
8-
},
9-
videoLocate(locator) {
10-
return locator ? locate(locator).inside(this.locateVideoContainer()) : this.locateVideoContainer();
11-
},
12-
};
16+
_trackSelector: '.lsf-seeker__track',
17+
_indicatorSelector: '.lsf-seeker__indicator',
18+
_positionSelector: '.lsf-seeker__position',
19+
_seekStepForwardSelector: '.lsf-timeline-controls__main-controls > div:nth-child(2) > button:nth-child(4)',
20+
_seekStepBackwardSelector: '.lsf-timeline-controls__main-controls > div:nth-child(2) > button:nth-child(2)',
21+
22+
locateVideoContainer() {
23+
return locate(this._videoRootSelector);
24+
},
25+
26+
videoLocate(locator) {
27+
return locator ? locate(locator).inside(this.locateVideoContainer()) : this.locateVideoContainer();
28+
},
29+
30+
/**
31+
* Grab the bounding rect of the video track
32+
* @returns {Promise<BoundingClientRect>}
33+
*/
34+
async grabTrackBoundingRect() {
35+
return I.grabElementBoundingRect(this._trackSelector);
36+
},
37+
38+
/**
39+
* Grab the bounding rect of the video indicator (the slider that outlines the viewable region)
40+
* @returns {Promise<BoundingClientRect>}
41+
*/
42+
async grabIndicatorBoundingRect() {
43+
return I.grabElementBoundingRect(this._indicatorSelector);
44+
},
45+
46+
/**
47+
* Grab the bounding rect of the video position (the playhead/cursor element).
48+
* @returns {Promise<BoundingClientRect>}
49+
*/
50+
async grabPositionBoundingRect() {
51+
return I.grabElementBoundingRect(this._positionSelector);
52+
},
53+
54+
/**
55+
* Drag the element to the given position
56+
* @param {BoundingClientRect} bbox
57+
* @param {number} x
58+
* @param {number} [y=undefined]
59+
* @returns {Promise<void>}
60+
*/
61+
async drag(bbox, x, y) {
62+
const from = { x: bbox.x + bbox.width / 2, y: bbox.y + bbox.height / 2 };
63+
const to = { x, y: y || from.y };
64+
65+
return I.dragAndDropMouse(from, to);
66+
},
67+
68+
/**
69+
* Seek forward steps
70+
* @param {number} steps
71+
* @returns {Promise<void>}
72+
*/
73+
async clickSeekStepForward(steps= 1) {
74+
for (let i = 0; i < steps; i++) {
75+
I.click(this._seekStepForwardSelector);
76+
}
77+
},
78+
79+
/**
80+
* Seek backward steps
81+
* @param {number} steps
82+
* @returns {Promise<void>}
83+
*/
84+
async clickSeekStepBackward(steps= 1) {
85+
for (let i = 0; i < steps; i++) {
86+
I.click(this._seekStepBackwardSelector);
87+
}
88+
},
89+
};

‎e2e/tests/regression-tests/video-timeline-seek-indicator.test.js‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/* global Feature, Scenario */
2+
3+
const assert = require('assert');
4+
5+
Feature('Video timeline seek indicator').tag('@regress');
6+
7+
8+
Scenario('Seek view should be in sync with indicator position', async ({ I, LabelStudio, AtVideoView }) => {
9+
I.amOnPage('/');
10+
LabelStudio.init({
11+
config: `
12+
<View>
13+
<Video name="video" value="$video" />
14+
<VideoRectangle name="box" toName="video" />
15+
16+
<Labels name="videoLabels" toName="video">
17+
<Label value="Car" background="#944BFF"/>
18+
<Label value="Airplane" background="#98C84E"/>
19+
<Label value="Possum" background="#FA8C16"/>
20+
</Labels>
21+
</View>`,
22+
data: { video: '/files/opossum_intro.webm' },
23+
});
24+
25+
I.say('waitForObjectsReady');
26+
LabelStudio.waitForObjectsReady();
27+
28+
const trackBbox = await AtVideoView.grabTrackBoundingRect();
29+
let positionBbox = await AtVideoView.grabPositionBoundingRect();
30+
let indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
31+
32+
assert.notDeepEqual({ x: 0, y: 0, width: 0, height: 0 }, trackBbox);
33+
assert.notDeepEqual({ x: 0, y: 0, width: 0, height: 0 }, positionBbox);
34+
assert.notDeepEqual({ x: 0, y: 0, width: 0, height: 0 }, indicatorBbox);
35+
36+
const trackZeroX = trackBbox.x;
37+
const halfway = (trackBbox.width - trackZeroX) / 2;
38+
39+
{
40+
I.say('Drag the video position indicator to the right to the middle');
41+
42+
AtVideoView.drag(positionBbox, halfway, 0);
43+
positionBbox = await AtVideoView.grabPositionBoundingRect();
44+
indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
45+
46+
I.say('Check the video position indicator is close to 50%');
47+
const delta = Math.round((positionBbox.x / (trackBbox.x + trackBbox.width)) * 10) / 10;
48+
49+
assert.equal(0.5, delta);
50+
51+
I.say('Check the video position indicator is within the seek indicator');
52+
assert.ok(indicatorBbox.x <= positionBbox.x, 'Video position indicator not within the seek indicator when dragged near the midway point');
53+
assert.ok(indicatorBbox.x + indicatorBbox.width >= positionBbox.x + positionBbox.width, 'Video position indicator not within the seek indicator when dragged near the midway point');
54+
}
55+
56+
{
57+
I.say('Drag the video position indicator to the end');
58+
AtVideoView.drag(positionBbox, trackBbox.width, 0);
59+
positionBbox = await AtVideoView.grabPositionBoundingRect();
60+
indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
61+
62+
I.say('Check the video position indicator is close to 100%');
63+
const delta = Math.round((positionBbox.x / (trackBbox.x + trackBbox.width)) * 10) / 10;
64+
65+
assert.equal(1, delta);
66+
67+
I.say('Check the video position indicator is within the seek indicator');
68+
assert.ok(indicatorBbox.x <= positionBbox.x, 'Video position indicator not within the seek indicator when dragged to the end');
69+
assert.ok(indicatorBbox.x + indicatorBbox.width >= positionBbox.x + positionBbox.width, 'Video position indicator not within the seek indicator when dragged to the end');
70+
}
71+
72+
{
73+
I.say('Drag the video position indicator to the left to the middle');
74+
AtVideoView.drag(positionBbox, halfway, 0);
75+
positionBbox = await AtVideoView.grabPositionBoundingRect();
76+
indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
77+
78+
I.say('Check the video position indicator is close to 50%');
79+
const delta = Math.round((positionBbox.x / (trackBbox.x + trackBbox.width)) * 10) / 10;
80+
81+
assert.equal(0.5, delta);
82+
83+
I.say('Check the video position indicator is within the seek indicator');
84+
assert.ok(indicatorBbox.x <= positionBbox.x, 'Video position indicator not within the seek indicator when dragged back to the midway point');
85+
assert.ok(indicatorBbox.x + indicatorBbox.width >= positionBbox.x + positionBbox.width, 'Video position indicator not within the seek indicator when dragged back to the midway point');
86+
}
87+
88+
{
89+
I.say('Drag the video position indicator to the start');
90+
AtVideoView.drag(positionBbox, 0, 0);
91+
positionBbox = await AtVideoView.grabPositionBoundingRect();
92+
indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
93+
94+
I.say('Check the video position indicator is close to 0%');
95+
const delta = Math.round((positionBbox.x / (trackBbox.x + trackBbox.width)) * 10) / 10;
96+
97+
assert.equal(0, delta);
98+
99+
I.say('Check the video position indicator is within the seek indicator');
100+
assert.ok(indicatorBbox.x <= positionBbox.x, 'Video position indicator not within the seek indicator when dragged back to the starting point');
101+
assert.ok(indicatorBbox.x + indicatorBbox.width >= positionBbox.x + positionBbox.width,'Video position indicator not within the seek indicator when dragged back to the starting point');
102+
}
103+
104+
{
105+
I.say('Click on the seek step forward button until the video position indicator is at the end');
106+
let indicatorPosX = indicatorBbox.x;
107+
108+
I.say('Move the video position indicator to the end of the seek indicator');
109+
const endOfSeeker = indicatorBbox.width - 2; // The indicator width is set wider by 1.5, to account for the pixel sizing of the position indicator width and placement rounding, so subtract this
110+
111+
await AtVideoView.drag(positionBbox, endOfSeeker, 0);
112+
indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
113+
114+
I.say('Seeker should not have moved');
115+
assert.equal(indicatorBbox.x, indicatorPosX, 'Seeker should not have moved from this one step movement');
116+
indicatorPosX = indicatorBbox.x;
117+
118+
I.say('Click on the seek step forward button');
119+
await AtVideoView.clickSeekStepForward(2);
120+
indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
121+
122+
I.say('Seeker should now have moved to the right');
123+
assert.ok(indicatorBbox.x > indicatorPosX, 'Seeker should have moved from this one step movement');
124+
indicatorPosX = indicatorBbox.x;
125+
126+
I.say('Click on the seek step backward button');
127+
await AtVideoView.clickSeekStepBackward(1);
128+
indicatorBbox = await AtVideoView.grabIndicatorBoundingRect();
129+
130+
I.say('Seeker should now have moved to the left');
131+
assert.ok(indicatorBbox.x < indicatorPosX, 'Seeker should have moved to the left from this one step movement');
132+
}
133+
});
134+

‎src/components/Timeline/Seeker.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@ export const Seeker: FC<SeekerProps> = ({
3434
const viewRef = useRef<HTMLDivElement>();
3535

3636
const showIndicator = seekVisible > 0;
37-
const width = `${(seekVisible - leftOffset) / length * 100}%`;
37+
38+
// The indicator width is set wider by 1.5, to account for the pixel sizing of the position indicator width and placement
39+
// to align better with the viewable timeline scroll.
40+
const width = `${(Math.ceil(seekVisible) - Math.floor(leftOffset) + 1.5) / length * 100}%`;
3841
const offsetLimit = length - (seekVisible - leftOffset);
3942
const windowOffset = `${Math.min(seekOffset, offsetLimit) / length * 100}%`;
4043
const seekerOffset = position / length * 100;

‎src/components/Timeline/SideControls/FramesControl.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export const FramesControl: FC<TimelineSideControlProps> = ({
2828
}}
2929
/>
3030
) : (
31-
<>{clamp(Math.round(position + 1), 1, duration)} <span>of {Math.round(duration)}</span></>
31+
<>{clamp(Math.round(position + 1), 1, duration + 1)} <span>of {duration + 1}</span></>
3232
)}
3333
</Block>
3434
);

‎src/components/Timeline/Timeline.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ const TimelineComponent: FC<TimelineProps> = ({
144144
onStepForward={increasePosition}
145145
onRewind={(steps) => setInternalPosition(isDefined(steps) ? currentPosition - steps : 0)}
146146
onForward={(steps) => setInternalPosition(isDefined(steps) ? currentPosition + steps : length)}
147-
onPositionChange={setInternalPosition}
147+
onPositionChange={setInternalPosition}
148148
onToggleCollapsed={setViewCollapsed}
149149
formatPosition={formatPosition}
150150
extraControls={View.Controls && !disableView ? (

0 commit comments

Comments
 (0)