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.

Sign up for GitHub

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

Jump to bottom

Add keyboard support for esc and seek for imavid #4726

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Add keyboard support for esc and seek for imavid #4726

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 74 additions & 51 deletions app/packages/looker/src/elements/common/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright 2017-2024, Voxel51, Inc.
*/

import { is } from "immutable";
import { SCALE_FACTOR } from "../../constants";
import { ImaVidFramesController } from "../../lookers/imavid/controller";
import {
Expand Down Expand Up @@ -553,23 +554,41 @@ export const resetPlaybackRate: Control<VideoState> = {
},
};

const seekTo: Control<VideoState> = {
const seekTo: Control<VideoState | ImaVidState> = {
title: "Seek to",
detail: "Seek to 0%, 10%, 20%... of the video",
shortcut: "0-9",
eventKeys: ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"],
action: (update, dispatchEvent, eventKey) => {
update(({ duration, config: { frameRate, support }, lockedToSupport }) => {
const frameCount = getFrameNumber(duration, duration, frameRate);
const total = lockedToSupport ? support[1] - support[0] : frameCount;
const base = lockedToSupport ? support[0] : 1;

update((state: ImaVidState | VideoState) => {
const isImavid = (state.config as ImaVidConfig)
.frameStoreController as ImaVidFramesController;
const frameName = isImavid ? "currentFrameNumber" : "frameNumber";
let total = 0;
let base = 0;
if (isImavid) {
const {
config: {
frameStoreController: { totalFrameCount },
},
currentFrameNumber,
} = state as ImaVidState;
total = totalFrameCount;
base = currentFrameNumber < totalFrameCount ? currentFrameNumber : 1;
} else {
const {
lockedToSupport,
config: { support, frameRate },
duration,
} = state as VideoState;
const frameCount = getFrameNumber(duration, duration, frameRate);
total = lockedToSupport ? support[1] - support[0] : frameCount;
base = lockedToSupport ? support[0] : 1;
}
const position = Math.round((parseInt(eventKey, 10) / 10) * total) + base;
dispatchEvent("options", { showJSON: false });
return {
frameNumber: Math.max(
1,
Math.round((parseInt(eventKey, 10) / 10) * total) + base
),
[frameName]: Math.min(total, Math.max(1, position)),
Comment on lines +557 to +591
Copy link
Contributor

Choose a reason for hiding this comment

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

Approve the seekTo control changes with a suggestion for readability.

The modifications to the seekTo control are well-aligned with the PR objectives and enhance the functionality by supporting both VideoState and ImaVidState. The logic to handle different state types and compute frame numbers based on the state is correctly implemented.

Suggestion for Improvement:
Consider adding comments within the action function to explain the logic, especially the calculations and conditions, to improve readability and maintainability.

options: { showJSON: false },
};
});
Expand All @@ -594,66 +613,70 @@ export const supportLock: Control<VideoState> = {
},
};

const videoEscape: Control<VideoState> = {
const videoEscape: Control<VideoState | ImaVidState> = {
title: "Escape context",
shortcut: "Esc",
eventKeys: "Escape",
detail: "Escape the current context",
alwaysHandle: true,
action: (update, dispatchEvent, eventKey) => {
update(
({
action: (update, dispatchEvent) => {
update((state: ImaVidState | VideoState) => {
const isImavid = (state.config as ImaVidConfig)
.frameStoreController as ImaVidFramesController;

const frameName = isImavid ? "currentFrameNumber" : "frameNumber";

const {
hasDefaultZoom,
showOptions,
frameNumber,
config: { support },
options: { showHelp, showJSON, selectedLabels },
lockedToSupport,
}) => {
if (showHelp) {
dispatchEvent("panels", { showHelp: "close" });
return { showHelp: "close" };
}
} = state as VideoState;

if (showOptions) {
return { showOptions: false };
}
if (showHelp) {
dispatchEvent("panels", { showHelp: "close" });
return { showHelp: "close" };
}

if (showJSON) {
dispatchEvent("panels", { showJSON: "close" });
dispatchEvent("options", { showJSON: false });
return { options: { showJSON: false } };
}
if (showOptions) {
return { showOptions: false };
}

if (!lockedToSupport && Boolean(support)) {
return {
frameNumber: support[0],
lockedToSupport: true,
};
}
if (showJSON) {
dispatchEvent("panels", { showJSON: "close" });
dispatchEvent("options", { showJSON: false });
return { options: { showJSON: false } };
}

if (!hasDefaultZoom) {
return {
setZoom: true,
};
}
if (!lockedToSupport && Boolean(support) && !isImavid) {
return {
frameNumber: support[0],
lockedToSupport: true,
};
}

if (frameNumber !== 1) {
return {
frameNumber: 1,
playing: false,
};
}
if (!hasDefaultZoom) {
return {
setZoom: true,
};
}

if (selectedLabels.length) {
dispatchEvent("clear");
return {};
}
if (state[frameName] !== 1) {
return {
[frameName]: 1,
playing: false,
};
}

dispatchEvent("close");
if (selectedLabels.length) {
dispatchEvent("clear");
return {};
}
);

dispatchEvent("close");
return {};
});
Comment on lines +616 to +679
Copy link
Contributor

Choose a reason for hiding this comment

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

Approve the videoEscape control changes with a suggestion for consistency.

The modifications to the videoEscape control enhance its functionality by supporting both VideoState and ImaVidState, allowing users to exit video playback and reset the state using the esc key. The comprehensive logic covers various scenarios and conditions, ensuring robust functionality.

Suggestion for Improvement:
Ensure consistency in the handling of different states and conditions. For example, the handling of showJSON could be streamlined to reduce redundancy in dispatching events and updating state.

},
};

Expand Down
Loading