From ceb62fdbfbeacd6d5a83b22191b8c277f2169c06 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 3 Dec 2024 13:21:38 +0000 Subject: [PATCH 01/15] feat: LEAP-1424: Open preview window for images in Grid view Open a simple modal with image from the data of clicked task. Allow to navigate between tasks visible in Grid view by Prev/Next buttons. Only one image is displayed right now. --- .../MainView/GridView/GridPreview.module.scss | 29 +++++ .../MainView/GridView/GridPreview.tsx | 102 ++++++++++++++++++ .../components/MainView/GridView/GridView.jsx | 83 ++++++++------ 3 files changed, 179 insertions(+), 35 deletions(-) create mode 100644 web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss create mode 100644 web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss new file mode 100644 index 000000000000..75ccce6dc8a9 --- /dev/null +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss @@ -0,0 +1,29 @@ +.controls { + display: flex; +} + +.controls > *:first-child { + margin-right: auto; +} + +.controls > *:last-child { + margin-left: auto; +} + +.container { + overflow: hidden; + width: 100%; + height: 100%; + position: relative; +} + +.image { + pointer-events: none; + user-select: none; + cursor: move; + width: 100%; + height: 100%; + object-fit: contain; + overflow: hidden; + max-height: calc(80vh - 120px); +} diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx new file mode 100644 index 000000000000..7b597a5d7b9a --- /dev/null +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -0,0 +1,102 @@ +import { observer } from "mobx-react"; +import type { PropsWithChildren } from "react"; +import { createContext, useCallback, useEffect, useRef, useState } from "react"; +import { modal } from "../../Common/Modal/Modal"; +import styles from "./GridPreview.module.scss"; + +type Task = { + id: number, + data: Record, +}; + +type GridViewContextType = { + tasks: Task[], + currentTaskId: number | null, + setCurrentTaskId: (id: number | null) => void, +}; + +type TaskModalProps = GridViewContextType; + +export const GridViewContext = createContext({ + tasks: [], + currentTaskId: null, + setCurrentTaskId: () => {}, +}); + +const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModalProps) => { + const index = tasks.findIndex(task => task.id === currentTaskId); + const task = tasks[index]; + + const goToNext = () => { + if (index < tasks.length - 1) { + setCurrentTaskId(tasks[index + 1].id); + } + }; + + const goToPrev = () => { + if (index > 0) { + setCurrentTaskId(tasks[index - 1].id); + } + }; + + if (!task) { + return null; + } + + return ( +
+
+ + {/* @todo other controls */} + +
+
+ Task Preview +
+
+ ); +}); + +type GridViewProviderProps = PropsWithChildren<{ + data: Task[]; +}>; + +export const GridViewProvider: React.FC = ({ children, data }) => { + const [currentTaskId, setCurrentTaskId] = useState(null); + const modalRef = useRef<{ update: (props: object) => void, close: () => void } | null>(); + + const onClose = useCallback(() => { + modalRef.current = null; + setCurrentTaskId(null); + }, []); + + useEffect(() => { + if (currentTaskId === null) { + modalRef.current?.close(); + return; + } + + if (!modalRef.current) { + modalRef.current = modal({ + title: "Task Preview", + style: { width: 800 }, + children: , + onHidden: onClose, + }); + } else { + modalRef.current.update({ + children: , + }); + } + }, [currentTaskId, data, onClose]); + + return ( + + {children} + + ); +}; \ No newline at end of file diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx index c303869cb05f..72fada6955b4 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx @@ -1,5 +1,5 @@ import { observer } from "mobx-react"; -import React from "react"; +import { useContext, useState } from "react"; import AutoSizer from "react-virtualized-auto-sizer"; import { FixedSizeGrid } from "react-window"; import InfiniteLoader from "react-window-infinite-loader"; @@ -8,9 +8,10 @@ import { Checkbox } from "@humansignal/ui"; import { Space } from "../../Common/Space/Space"; import { getProperty, prepareColumns } from "../../Common/Table/utils"; import * as DataGroups from "../../DataGroups"; -import "./GridView.scss"; import { FF_LOPS_E_3, isFF } from "../../../utils/feature-flags"; import { SkeletonLoader } from "../../Common/SkeletonLoader"; +import { GridViewContext, GridViewProvider } from "./GridPreview"; +import "./GridView.scss"; const GridHeader = observer(({ row, selected }) => { const isSelected = selected.isSelected(row.id); @@ -56,11 +57,21 @@ const GridDataGroup = observer(({ type, value, field, row }) => { }); const GridCell = observer(({ view, selected, row, fields, onClick, ...props }) => { + const { setCurrentTaskId } = useContext(GridViewContext); + + const handleBodyClick = (e) => { + // @todo skip this interaction if there are no images in the task + e.stopPropagation(); + setCurrentTaskId(row.id); + }; + return ( - + + + ); @@ -135,37 +146,39 @@ export const GridView = observer(({ data, view, loadMore, fields, onChange, hidd ); return ( - - - {({ width, height }) => ( - - {({ onItemsRendered, ref }) => ( - - {renderItem} - - )} - - )} - - + + + + {({ width, height }) => ( + + {({ onItemsRendered, ref }) => ( + + {renderItem} + + )} + + )} + + + ); }); From 0dd939d5ade557fbf4dd84fa25b7efce52abe5a3 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 3 Dec 2024 13:42:51 +0000 Subject: [PATCH 02/15] Fix missing react hooks imports --- .../src/components/MainView/GridView/GridView.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx index 72fada6955b4..d9e1f9db58ba 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx @@ -1,5 +1,5 @@ import { observer } from "mobx-react"; -import { useContext, useState } from "react"; +import { useCallback, useContext, useMemo, useState } from "react"; import AutoSizer from "react-virtualized-auto-sizer"; import { FixedSizeGrid } from "react-window"; import InfiniteLoader from "react-window-infinite-loader"; @@ -82,7 +82,7 @@ export const GridView = observer(({ data, view, loadMore, fields, onChange, hidd const getCellIndex = (row, column) => columnCount * row + column; - const fieldsData = React.useMemo(() => { + const fieldsData = useMemo(() => { return prepareColumns(fields, hiddenFields); }, [fields, hiddenFields]); @@ -94,7 +94,7 @@ export const GridView = observer(({ data, view, loadMore, fields, onChange, hidd return res + height; }, 16); - const renderItem = React.useCallback( + const renderItem = useCallback( ({ style, rowIndex, columnIndex }) => { const index = getCellIndex(rowIndex, columnIndex); const row = data[index]; @@ -135,7 +135,7 @@ export const GridView = observer(({ data, view, loadMore, fields, onChange, hidd const itemCount = Math.ceil(data.length / columnCount); - const isItemLoaded = React.useCallback( + const isItemLoaded = useCallback( (index) => { const rowIndex = index * columnCount; const rowFullfilled = data.slice(rowIndex, columnCount).length === columnCount; From c7ae398770f41cf7d6f747754b70ad072b9cd81a Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 3 Dec 2024 14:53:30 +0000 Subject: [PATCH 03/15] Add hotkeys for navigation between pages --- .../MainView/GridView/GridPreview.tsx | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 7b597a5d7b9a..47364f673de5 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -26,6 +26,7 @@ export const GridViewContext = createContext({ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModalProps) => { const index = tasks.findIndex(task => task.id === currentTaskId); const task = tasks[index]; + const src = task?.data?.image; const goToNext = () => { if (index < tasks.length - 1) { @@ -39,6 +40,20 @@ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModa } }; + // assign hotkeys + useEffect(() => { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === "ArrowLeft") { + goToPrev(); + } else if (event.key === "ArrowRight") { + goToNext(); + } + }; + + document.addEventListener("keydown", onKeyDown); + return () => document.removeEventListener("keydown", onKeyDown); + }, [goToNext, goToPrev]); + if (!task) { return null; } @@ -53,7 +68,7 @@ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModa
Task Preview
@@ -99,4 +114,4 @@ export const GridViewProvider: React.FC = ({ children, da {children} ); -}; \ No newline at end of file +}; From 5eb7e44dd7f4ef6d0ea459a1b7ed689655bdc649 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 3 Dec 2024 15:43:39 +0000 Subject: [PATCH 04/15] Small comment about key and image flickering --- .../src/components/MainView/GridView/GridPreview.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 47364f673de5..1320305a4626 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -67,6 +67,9 @@ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModa
Task Preview Date: Tue, 3 Dec 2024 19:40:59 +0000 Subject: [PATCH 05/15] Add more hotkeys, improve appearance, add tooltip Modal is `bare` now to allow to make better styles for it. UI is more dense and different elements are aligned. --- .../MainView/GridView/GridPreview.module.scss | 49 ++++++++++++-- .../MainView/GridView/GridPreview.tsx | 67 +++++++++++++++---- .../components/MainView/GridView/GridView.jsx | 2 +- 3 files changed, 99 insertions(+), 19 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss index 75ccce6dc8a9..df3ca960d29c 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.module.scss @@ -1,22 +1,61 @@ -.controls { +.modal { + padding: 16px; + position: relative; +} + +.header { display: flex; + justify-content: space-between; + align-items: center; + box-sizing: content-box; + margin-bottom: 16px; } -.controls > *:first-child { - margin-right: auto; +.tooltip { + font-size: 12px; + max-width: 300px; + line-height: 16px; + + p { + margin-bottom: 4px; + } + + p:last-child { + margin-bottom: 0; + } } -.controls > *:last-child { +.actions { margin-left: auto; + + & > * { + width: 20px; + margin-left: 16px; + text-align: center; + cursor: pointer; + } } .container { overflow: hidden; width: 100%; height: 100%; + min-height: 100px; + display: flex; position: relative; } +.container button { + padding: 0; + flex: 20px 0 0; + cursor: pointer; + background: none; + + &:hover { + background: var(--sand_200); + } +} + .image { pointer-events: none; user-select: none; @@ -25,5 +64,5 @@ height: 100%; object-fit: contain; overflow: hidden; - max-height: calc(80vh - 120px); + max-height: calc(90vh - 120px); } diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 1320305a4626..28d332e3b76f 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -1,7 +1,11 @@ +import { CloseOutlined, LeftCircleOutlined, QuestionCircleOutlined, RightCircleOutlined } from "@ant-design/icons"; +import { Checkbox } from "@humansignal/ui"; import { observer } from "mobx-react"; import type { PropsWithChildren } from "react"; import { createContext, useCallback, useEffect, useRef, useState } from "react"; import { modal } from "../../Common/Modal/Modal"; +import { Icon } from "../../Common/Icon/Icon"; +import { Tooltip } from "../../Common/Tooltip/Tooltip"; import styles from "./GridPreview.module.scss"; type Task = { @@ -15,7 +19,7 @@ type GridViewContextType = { setCurrentTaskId: (id: number | null) => void, }; -type TaskModalProps = GridViewContextType; +type TaskModalProps = GridViewContextType & { view: any }; export const GridViewContext = createContext({ tasks: [], @@ -23,7 +27,7 @@ export const GridViewContext = createContext({ setCurrentTaskId: () => {}, }); -const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModalProps) => { +const TaskModal = observer(({ view, tasks, currentTaskId, setCurrentTaskId }: TaskModalProps) => { const index = tasks.findIndex(task => task.id === currentTaskId); const task = tasks[index]; const src = task?.data?.image; @@ -40,6 +44,10 @@ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModa } }; + const onSelect = () => view.toggleSelected(task.id); + + const onClose = () => setCurrentTaskId(null); + // assign hotkeys useEffect(() => { const onKeyDown = (event: KeyboardEvent) => { @@ -47,7 +55,17 @@ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModa goToPrev(); } else if (event.key === "ArrowRight") { goToNext(); + } else if (event.key === " ") { + onSelect(); + event.preventDefault(); + } else if (event.key === "Escape") { + onClose(); + } else { + // pass this event through for other keys + return; } + + event.stopPropagation(); }; document.addEventListener("keydown", onKeyDown); @@ -58,22 +76,43 @@ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModa return null; } + const tooltip = ( +
+

Preview of the task image to quickly navigate through the tasks and select the ones you want to work on.

+

Use [arrow keys] to navigate.

+

[Escape] to close the modal.

+

[Space] to select/unselect the task.

+
+ ) + return ( -
-
- - {/* @todo other controls */} - +
+
+ + Task {task.id} + +
+ + + + +
+ Task Preview +
); @@ -81,9 +120,10 @@ const TaskModal = observer(({ tasks, currentTaskId, setCurrentTaskId }: TaskModa type GridViewProviderProps = PropsWithChildren<{ data: Task[]; + view: any; }>; -export const GridViewProvider: React.FC = ({ children, data }) => { +export const GridViewProvider: React.FC = ({ children, data, view }) => { const [currentTaskId, setCurrentTaskId] = useState(null); const modalRef = useRef<{ update: (props: object) => void, close: () => void } | null>(); @@ -98,17 +138,18 @@ export const GridViewProvider: React.FC = ({ children, da return; } + const children = ; + if (!modalRef.current) { modalRef.current = modal({ + bare: true, title: "Task Preview", style: { width: 800 }, - children: , + children, onHidden: onClose, }); } else { - modalRef.current.update({ - children: , - }); + modalRef.current.update({ children }); } }, [currentTaskId, data, onClose]); diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx index d9e1f9db58ba..1fff3fe9fb76 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx @@ -146,7 +146,7 @@ export const GridView = observer(({ data, view, loadMore, fields, onChange, hidd ); return ( - + {({ width, height }) => ( From 9ca7d19823b845e03e94748e1843e49ace030ac8 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Sat, 7 Dec 2024 18:34:25 +0000 Subject: [PATCH 06/15] Add FF to control preview in Grid View --- .../datamanager/src/components/MainView/GridView/GridView.jsx | 3 ++- web/libs/datamanager/src/utils/feature-flags.js | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx index 1fff3fe9fb76..825cbdad4aa7 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx @@ -8,7 +8,7 @@ import { Checkbox } from "@humansignal/ui"; import { Space } from "../../Common/Space/Space"; import { getProperty, prepareColumns } from "../../Common/Table/utils"; import * as DataGroups from "../../DataGroups"; -import { FF_LOPS_E_3, isFF } from "../../../utils/feature-flags"; +import { FF_GRID_PREVIEW, FF_LOPS_E_3, isFF } from "../../../utils/feature-flags"; import { SkeletonLoader } from "../../Common/SkeletonLoader"; import { GridViewContext, GridViewProvider } from "./GridPreview"; import "./GridView.scss"; @@ -60,6 +60,7 @@ const GridCell = observer(({ view, selected, row, fields, onClick, ...props }) = const { setCurrentTaskId } = useContext(GridViewContext); const handleBodyClick = (e) => { + if (!isFF(FF_GRID_PREVIEW)) return; // @todo skip this interaction if there are no images in the task e.stopPropagation(); setCurrentTaskId(row.id); diff --git a/web/libs/datamanager/src/utils/feature-flags.js b/web/libs/datamanager/src/utils/feature-flags.js index 6e3f7fe1716d..94d5f685c0fd 100644 --- a/web/libs/datamanager/src/utils/feature-flags.js +++ b/web/libs/datamanager/src/utils/feature-flags.js @@ -71,6 +71,9 @@ export const FF_LOPS_86 = "fflag_feat_front_lops_86_datasets_storage_edit_short" */ export const FF_SELF_SERVE = "fflag_feat_front_leap_482_self_serve_short"; +/** Add ability to preview image tasks in Data Manager Grid View */ +export const FF_GRID_PREVIEW = "fflag_feat_front_leap_1424_grid_preview_short"; + // Customize flags const flags = {}; From e2799684b17a2c6f73d21d52d0dd136ea6ea0e3d Mon Sep 17 00:00:00 2001 From: hlomzik Date: Sat, 7 Dec 2024 18:36:53 +0000 Subject: [PATCH 07/15] Show preview modal only if there is an image in a task Check fields for "Image" type and show modal only if it exists. --- .../components/MainView/GridView/GridPreview.tsx | 16 +++++++++++----- .../components/MainView/GridView/GridView.jsx | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 28d332e3b76f..02be4129b506 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -15,6 +15,7 @@ type Task = { type GridViewContextType = { tasks: Task[], + imageField: string | undefined, currentTaskId: number | null, setCurrentTaskId: (id: number | null) => void, }; @@ -23,14 +24,15 @@ type TaskModalProps = GridViewContextType & { view: any }; export const GridViewContext = createContext({ tasks: [], + imageField: undefined, currentTaskId: null, setCurrentTaskId: () => {}, }); -const TaskModal = observer(({ view, tasks, currentTaskId, setCurrentTaskId }: TaskModalProps) => { +const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrentTaskId }: TaskModalProps) => { const index = tasks.findIndex(task => task.id === currentTaskId); const task = tasks[index]; - const src = task?.data?.image; + const src = imageField ? (task?.data?.[imageField] || "") : ""; const goToNext = () => { if (index < tasks.length - 1) { @@ -121,11 +123,13 @@ const TaskModal = observer(({ view, tasks, currentTaskId, setCurrentTaskId }: Ta type GridViewProviderProps = PropsWithChildren<{ data: Task[]; view: any; + fields: { alias: string, currentType: string }[]; }>; -export const GridViewProvider: React.FC = ({ children, data, view }) => { +export const GridViewProvider: React.FC = ({ children, data, view, fields }) => { const [currentTaskId, setCurrentTaskId] = useState(null); const modalRef = useRef<{ update: (props: object) => void, close: () => void } | null>(); + const imageField = fields.find(f => f.currentType === "Image")?.alias; const onClose = useCallback(() => { modalRef.current = null; @@ -138,7 +142,9 @@ export const GridViewProvider: React.FC = ({ children, da return; } - const children = ; + if (!imageField) return; + + const children = ; if (!modalRef.current) { modalRef.current = modal({ @@ -154,7 +160,7 @@ export const GridViewProvider: React.FC = ({ children, da }, [currentTaskId, data, onClose]); return ( - + {children} ); diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx index 825cbdad4aa7..acfa5a181560 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx @@ -57,10 +57,10 @@ const GridDataGroup = observer(({ type, value, field, row }) => { }); const GridCell = observer(({ view, selected, row, fields, onClick, ...props }) => { - const { setCurrentTaskId } = useContext(GridViewContext); + const { setCurrentTaskId, imageField } = useContext(GridViewContext); const handleBodyClick = (e) => { - if (!isFF(FF_GRID_PREVIEW)) return; + if (!isFF(FF_GRID_PREVIEW) || !imageField) return; // @todo skip this interaction if there are no images in the task e.stopPropagation(); setCurrentTaskId(row.id); @@ -147,7 +147,7 @@ export const GridView = observer(({ data, view, loadMore, fields, onChange, hidd ); return ( - + {({ width, height }) => ( From 30292447e50abd8f692589c934a7834074b0e9a9 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Sat, 7 Dec 2024 18:38:13 +0000 Subject: [PATCH 08/15] Simple code improvement --- web/libs/datamanager/src/components/Common/Table/utils.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/libs/datamanager/src/components/Common/Table/utils.js b/web/libs/datamanager/src/components/Common/Table/utils.js index d80bc8aab96e..592fbcccae4a 100644 --- a/web/libs/datamanager/src/components/Common/Table/utils.js +++ b/web/libs/datamanager/src/components/Common/Table/utils.js @@ -1,6 +1,7 @@ export const prepareColumns = (columns, hidden) => { + if (!hidden?.length) return columns; return columns.filter((col) => { - return !(hidden ?? []).includes(col.id); + return !hidden.includes(col.id); }); }; From d57dad74b2dce89eff54bcc8473c2d97382f34be Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 10 Dec 2024 14:36:30 +0000 Subject: [PATCH 09/15] Wrap methods in `useCallback()` --- .../MainView/GridView/GridPreview.tsx | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 02be4129b506..e0c97af1a52c 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -34,21 +34,27 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent const task = tasks[index]; const src = imageField ? (task?.data?.[imageField] || "") : ""; - const goToNext = () => { + const goToNext = useCallback(() => { if (index < tasks.length - 1) { setCurrentTaskId(tasks[index + 1].id); } - }; + }, [index, tasks]); - const goToPrev = () => { + const goToPrev = useCallback(() => { if (index > 0) { setCurrentTaskId(tasks[index - 1].id); } - }; + }, [index, tasks]); - const onSelect = () => view.toggleSelected(task.id); + const onSelect = useCallback(() => { + if (task) { + view.toggleSelected(task.id); + } + }, [task, view]); - const onClose = () => setCurrentTaskId(null); + const onClose = useCallback(() => { + setCurrentTaskId(null); + }, []); // assign hotkeys useEffect(() => { @@ -72,7 +78,7 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent document.addEventListener("keydown", onKeyDown); return () => document.removeEventListener("keydown", onKeyDown); - }, [goToNext, goToPrev]); + }, [goToNext, goToPrev, onSelect, onClose]); if (!task) { return null; @@ -85,7 +91,7 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent

[Escape] to close the modal.

[Space] to select/unselect the task.

- ) + ); return (
From 2ca9ee05bc6c58b77d869d18fac19d97689f224e Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 10 Dec 2024 14:38:35 +0000 Subject: [PATCH 10/15] Wrap one more function with `useCallback()` Co-authored-by: bmartel --- .../datamanager/src/components/MainView/GridView/GridView.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx index acfa5a181560..33bc2573046f 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx @@ -59,12 +59,12 @@ const GridDataGroup = observer(({ type, value, field, row }) => { const GridCell = observer(({ view, selected, row, fields, onClick, ...props }) => { const { setCurrentTaskId, imageField } = useContext(GridViewContext); - const handleBodyClick = (e) => { + const handleBodyClick = useCallback((e) => { if (!isFF(FF_GRID_PREVIEW) || !imageField) return; // @todo skip this interaction if there are no images in the task e.stopPropagation(); setCurrentTaskId(row.id); - }; + }, [imageField, row.id]); return ( From d425b4216c8d3105029649eba8bd2b10400e982d Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 12 Dec 2024 00:38:56 +0000 Subject: [PATCH 11/15] Prevent Quick View from opening in a background by hotkey Apparently `Shift+Up` opens up a Quick View even in a background --- .../src/components/MainView/GridView/GridPreview.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index e0c97af1a52c..9285a730b166 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -68,6 +68,8 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent event.preventDefault(); } else if (event.key === "Escape") { onClose(); + } else if (event.key === "ArrowUp") { + // prevent Quick View from opening in a background by hotkey } else { // pass this event through for other keys return; From 360902533f55267bae3e10ee07ce35ac724d3682 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 12 Dec 2024 15:27:19 +0000 Subject: [PATCH 12/15] Fix hotkeys interception for background hotkeys --- .../src/components/MainView/GridView/GridPreview.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 9285a730b166..024a6a043042 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -68,7 +68,7 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent event.preventDefault(); } else if (event.key === "Escape") { onClose(); - } else if (event.key === "ArrowUp") { + } else if (event.key === "ArrowUp" || event.key === "ArrowDown") { // prevent Quick View from opening in a background by hotkey } else { // pass this event through for other keys @@ -78,7 +78,7 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent event.stopPropagation(); }; - document.addEventListener("keydown", onKeyDown); + document.addEventListener("keydown", onKeyDown, { capture: true }); return () => document.removeEventListener("keydown", onKeyDown); }, [goToNext, goToPrev, onSelect, onClose]); From fb65a2a29305e097cbcbde92bf7ea9eb073e9f8b Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 13 Dec 2024 14:34:37 +0000 Subject: [PATCH 13/15] Properly handle view change When we leave the Grid View by browser controls or by hotkeys we need to close the modal. That also means that we don't need to intercept Quick View hotkey. And `capture` was a huge hit for performance, because keydown handler was called million times during handling the same event, leading to browser freeze. --- .../src/components/MainView/GridView/GridPreview.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 024a6a043042..7307ed5434a8 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -68,8 +68,6 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent event.preventDefault(); } else if (event.key === "Escape") { onClose(); - } else if (event.key === "ArrowUp" || event.key === "ArrowDown") { - // prevent Quick View from opening in a background by hotkey } else { // pass this event through for other keys return; @@ -78,7 +76,7 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent event.stopPropagation(); }; - document.addEventListener("keydown", onKeyDown, { capture: true }); + document.addEventListener("keydown", onKeyDown); return () => document.removeEventListener("keydown", onKeyDown); }, [goToNext, goToPrev, onSelect, onClose]); @@ -136,7 +134,7 @@ type GridViewProviderProps = PropsWithChildren<{ export const GridViewProvider: React.FC = ({ children, data, view, fields }) => { const [currentTaskId, setCurrentTaskId] = useState(null); - const modalRef = useRef<{ update: (props: object) => void, close: () => void } | null>(); + const modalRef = useRef<{ update: (props: object) => void, close: () => void } | null>(null); const imageField = fields.find(f => f.currentType === "Image")?.alias; const onClose = useCallback(() => { @@ -167,6 +165,9 @@ export const GridViewProvider: React.FC = ({ children, da } }, [currentTaskId, data, onClose]); + // close the modal when we leave the view (by browser controls or by hotkeys) + useEffect(() => () => modalRef.current?.close()); + return ( {children} From a1664004b07aecc8784f701486b84a588c846082 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 13 Dec 2024 14:37:46 +0000 Subject: [PATCH 14/15] Fix linting --- .../MainView/GridView/GridPreview.tsx | 32 ++++++++++++------- .../components/MainView/GridView/GridView.jsx | 17 ++++++---- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 7307ed5434a8..21cdf4e724b1 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -9,15 +9,15 @@ import { Tooltip } from "../../Common/Tooltip/Tooltip"; import styles from "./GridPreview.module.scss"; type Task = { - id: number, - data: Record, + id: number; + data: Record; }; type GridViewContextType = { - tasks: Task[], - imageField: string | undefined, - currentTaskId: number | null, - setCurrentTaskId: (id: number | null) => void, + tasks: Task[]; + imageField: string | undefined; + currentTaskId: number | null; + setCurrentTaskId: (id: number | null) => void; }; type TaskModalProps = GridViewContextType & { view: any }; @@ -30,9 +30,9 @@ export const GridViewContext = createContext({ }); const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrentTaskId }: TaskModalProps) => { - const index = tasks.findIndex(task => task.id === currentTaskId); + const index = tasks.findIndex((task) => task.id === currentTaskId); const task = tasks[index]; - const src = imageField ? (task?.data?.[imageField] || "") : ""; + const src = imageField ? task?.data?.[imageField] || "" : ""; const goToNext = useCallback(() => { if (index < tasks.length - 1) { @@ -129,13 +129,13 @@ const TaskModal = observer(({ view, tasks, imageField, currentTaskId, setCurrent type GridViewProviderProps = PropsWithChildren<{ data: Task[]; view: any; - fields: { alias: string, currentType: string }[]; + fields: { alias: string; currentType: string }[]; }>; export const GridViewProvider: React.FC = ({ children, data, view, fields }) => { const [currentTaskId, setCurrentTaskId] = useState(null); - const modalRef = useRef<{ update: (props: object) => void, close: () => void } | null>(null); - const imageField = fields.find(f => f.currentType === "Image")?.alias; + const modalRef = useRef<{ update: (props: object) => void; close: () => void } | null>(null); + const imageField = fields.find((f) => f.currentType === "Image")?.alias; const onClose = useCallback(() => { modalRef.current = null; @@ -150,7 +150,15 @@ export const GridViewProvider: React.FC = ({ children, da if (!imageField) return; - const children = ; + const children = ( + + ); if (!modalRef.current) { modalRef.current = modal({ diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx index 33bc2573046f..d5a6e3e43128 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridView.jsx @@ -1,5 +1,5 @@ import { observer } from "mobx-react"; -import { useCallback, useContext, useMemo, useState } from "react"; +import { useCallback, useContext, useMemo } from "react"; import AutoSizer from "react-virtualized-auto-sizer"; import { FixedSizeGrid } from "react-window"; import InfiniteLoader from "react-window-infinite-loader"; @@ -59,12 +59,15 @@ const GridDataGroup = observer(({ type, value, field, row }) => { const GridCell = observer(({ view, selected, row, fields, onClick, ...props }) => { const { setCurrentTaskId, imageField } = useContext(GridViewContext); - const handleBodyClick = useCallback((e) => { - if (!isFF(FF_GRID_PREVIEW) || !imageField) return; - // @todo skip this interaction if there are no images in the task - e.stopPropagation(); - setCurrentTaskId(row.id); - }, [imageField, row.id]); + const handleBodyClick = useCallback( + (e) => { + if (!isFF(FF_GRID_PREVIEW) || !imageField) return; + // @todo skip this interaction if there are no images in the task + e.stopPropagation(); + setCurrentTaskId(row.id); + }, + [imageField, row.id], + ); return ( From 88e74c1cdf49545425d614c25d375555ebef9529 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Fri, 13 Dec 2024 15:40:45 +0000 Subject: [PATCH 15/15] Fix silly mistake useEffect closing the modal should be global one-time one --- .../src/components/MainView/GridView/GridPreview.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx index 21cdf4e724b1..6428cc47e4c0 100644 --- a/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx +++ b/web/libs/datamanager/src/components/MainView/GridView/GridPreview.tsx @@ -174,7 +174,7 @@ export const GridViewProvider: React.FC = ({ children, da }, [currentTaskId, data, onClose]); // close the modal when we leave the view (by browser controls or by hotkeys) - useEffect(() => () => modalRef.current?.close()); + useEffect(() => () => modalRef.current?.close(), []); return (