Merge pull request #4925 from voxel51/fix/shortcuts

fix shortcuts crashing + add next/previous shortcuts in help modal

app/packages/core/src/components/Modal/utils.test.ts

@@ -0,0 +1,12 @@
+import { describe, expect, it } from "vitest";
+import { shortcutToHelpItems } from "./utils";
+
+describe("shortcut processing test", () => {
+  it("parses unique shortcuts", () => {
+    const results = shortcutToHelpItems({
+      one: { shortcut: "test" },
+      two: { shortcut: "test" },
+    });
+    expect(results).toStrictEqual([{ shortcut: "test" }]);
+  });
+});

app/packages/core/src/components/Modal/utils.ts

@@ -1,7 +1,13 @@
-export function shortcutToHelpItems(SHORTCUTS) {
-  const result = {};
-  for (const k of SHORTCUTS) {
-    result[SHORTCUTS[k].shortcut] = SHORTCUTS[k];
+interface ShortcutItem {
+  shortcut: string;
+}
+
+type Shortcuts = { [key: string]: ShortcutItem };
+
+export function shortcutToHelpItems(SHORTCUTS: Shortcuts) {
+  const uniqueItems = {};
+  for (const item of Object.values(SHORTCUTS)) {
+    uniqueItems[item.shortcut] = item;
   }
-  return Object.values(result);
+  return Object.values(uniqueItems);
 }

app/packages/looker/src/elements/common/actions.ts

@@ -405,6 +405,17 @@ export const nextFrame: Control<VideoState> = {
   },
 };
 
+export const nextFrameNoOpControl: Control<ImaVidState> = {
+  title: "Next frame",
+  eventKeys: [".", ">"],
+  shortcut: ">",
+  detail: "Seek to the next frame",
+  alwaysHandle: true,
+  action: () => {
+    // no-op here, supposed to be implemented elsewhere
+  },
+};
+
 export const previousFrame: Control<VideoState> = {
   title: "Previous frame",
   eventKeys: [",", "<"],
@@ -436,6 +447,17 @@ export const previousFrame: Control<VideoState> = {
   },
 };
 
+export const previousFrameNoOpControl: Control<ImaVidState> = {
+  title: "Previous frame",
+  eventKeys: [",", "<"],
+  shortcut: "<",
+  detail: "Seek to the previous frame",
+  alwaysHandle: true,
+  action: () => {
+    // no-op here, supposed to be implemented elsewhere
+  },
+};
+
 export const playPause: Control<VideoState> = {
   title: "Play / pause",
   shortcut: "Space",
@@ -662,6 +684,8 @@ const VIDEO = {
 const IMAVID = {
   ...COMMON,
   escape: videoEscape,
+  previousFrame: previousFrameNoOpControl,
+  nextFrame: nextFrameNoOpControl,
 };
 
 export const VIDEO_SHORTCUTS = readActions(VIDEO);

app/packages/looker/src/lookers/image.ts

@@ -5,6 +5,10 @@ import { DEFAULT_IMAGE_OPTIONS, ImageState } from "../state";
 import { AbstractLooker } from "./abstract";
 import { LookerUtils } from "./shared";
 
+import {
+  nextFrameNoOpControl,
+  previousFrameNoOpControl,
+} from "../elements/common/actions";
 import { zoomToContent } from "../zoom";
 
 export class ImageLooker extends AbstractLooker<ImageState> {
@@ -21,11 +25,21 @@ export class ImageLooker extends AbstractLooker<ImageState> {
       ...options,
     };
 
+    // if in dynamic groups mode, add < > shortcuts, too
+    let shortcuts = { ...COMMON_SHORTCUTS };
+    if (config.isDynamicGroup) {
+      shortcuts = {
+        ...COMMON_SHORTCUTS,
+        previousFrameNoOpControl,
+        nextFrameNoOpControl,
+      };
+    }
+
     return {
       ...this.getInitialBaseState(),
       config: { ...config },
       options,
-      SHORTCUTS: COMMON_SHORTCUTS,
+      SHORTCUTS: shortcuts,
     };
   }
 

app/packages/looker/src/state.ts

@@ -203,6 +203,7 @@ export interface BaseConfig {
   sampleId: string;
   symbol: symbol;
   fieldSchema: Schema;
+  isDynamicGroup: boolean;
   view: Stage[];
   dataset: string;
   group?: {

app/packages/state/src/hooks/useCreateLooker.ts

@@ -61,6 +61,8 @@ export default <T extends AbstractLooker<BaseState>>(
     dynamicGroupAtoms.shouldRenderImaVidLooker(isModal)
   );
 
+  const isDynamicGroup = useRecoilValue(dynamicGroupAtoms.isDynamicGroup);
+
   // callback to get the latest promise inside another recoil callback
   // gets around the limitation of the fact that snapshot inside callback refs to the committed state at the time
   const getPromise = useRecoilCallback(
@@ -128,6 +130,7 @@ export default <T extends AbstractLooker<BaseState>>(
           sources: urls,
           frameNumber: create === FrameLooker ? frameNumber : undefined,
           frameRate,
+          isDynamicGroup,
           sampleId: sample._id,
           support: isClip ? sample.support : undefined,
           dataset,