From 016b96307ac89fdf0bec6dd0bd4649a681217dad Mon Sep 17 00:00:00 2001 From: imanjra Date: Thu, 5 Sep 2024 09:07:29 -0400 Subject: [PATCH 1/2] fixes and enhancements for modal spaces --- .../src/components/Actions/ActionsRow.tsx | 14 +-------- .../core/src/components/Modal/Modal.tsx | 6 ++-- .../src/components/Modal/ModalNavigation.tsx | 2 +- .../components/Modal/ModalSamplePlugin.tsx | 20 +++++++++++- .../core/src/components/Sidebar/Sidebar.tsx | 1 + .../SchemaIO/components/DashboardView.tsx | 4 +-- .../core/src/plugins/modal-sample.tsx | 2 ++ .../operators/src/useCustomPanelHooks.ts | 7 +++-- .../spaces/src/components/AddPanelButton.tsx | 31 +++++++------------ .../spaces/src/components/StyledElements.tsx | 11 ++----- 10 files changed, 48 insertions(+), 50 deletions(-) diff --git a/app/packages/core/src/components/Actions/ActionsRow.tsx b/app/packages/core/src/components/Actions/ActionsRow.tsx index 551d607ddc..02be833734 100644 --- a/app/packages/core/src/components/Actions/ActionsRow.tsx +++ b/app/packages/core/src/components/Actions/ActionsRow.tsx @@ -554,18 +554,6 @@ const ModalActionsRowContainer = styled.div` opacity: 1; transition: opacity 0.1s ease-out; } - - svg { - font-size: 14px; - } - - > div { - max-height: 21px; - - > div:first-child { - max-height: 21px; - } - } }`; const DraggableHandleIconContainer = styled.div` @@ -750,7 +738,7 @@ export const ModalActionsRow = () => { handle={`.${MODAL_ACTION_BAR_HANDLE_CLASS}`} axis="x" defaultPosition={{ x: defaultXCoord ?? 0, y: 0 }} - onDrag={(_e, { x, y }) => { + onDrag={(_e, { x }) => { setDefaultXCoord(x); }} > diff --git a/app/packages/core/src/components/Modal/Modal.tsx b/app/packages/core/src/components/Modal/Modal.tsx index 5dd8f548ac..db3b97e03b 100644 --- a/app/packages/core/src/components/Modal/Modal.tsx +++ b/app/packages/core/src/components/Modal/Modal.tsx @@ -9,7 +9,6 @@ import { ModalActionsRow } from "../Actions"; import Sidebar from "../Sidebar"; import { useLookerHelpers } from "./hooks"; import { modalContext } from "./modal-context"; -import ModalNavigation from "./ModalNavigation"; import { ModalSpace } from "./ModalSpace"; import { TooltipInfo } from "./TooltipInfo"; import { useModalSidebarRenderEntry } from "./use-sidebar-render-entry"; @@ -44,6 +43,7 @@ const SpacesContainer = styled.div` display: flex; flex-direction: column; overflow: hidden; + z-index: 1501; `; const SidebarPanelBlendInDiv = styled.div` @@ -77,7 +77,7 @@ const Modal = () => { const renderEntry = useModalSidebarRenderEntry(); - const { jsonPanel, helpPanel, onNavigate } = useLookerHelpers(); + const { jsonPanel, helpPanel } = useLookerHelpers(); const select = fos.useSelectSample(); @@ -166,7 +166,7 @@ const Modal = () => { const screenParams = useMemo(() => { return isFullScreen ? { width: "100%", height: "100%" } - : { width: "95%", height: "90%", borderRadius: "3px" }; + : { width: "95%", height: "calc(100% - 70px)", borderRadius: "8px" }; }, [isFullScreen]); const activeLookerRef = useRef(); diff --git a/app/packages/core/src/components/Modal/ModalNavigation.tsx b/app/packages/core/src/components/Modal/ModalNavigation.tsx index 2c2ca9012c..94f2441ada 100644 --- a/app/packages/core/src/components/Modal/ModalNavigation.tsx +++ b/app/packages/core/src/components/Modal/ModalNavigation.tsx @@ -26,7 +26,7 @@ const Arrow = styled.span<{ left: ${(props) => (props.$isRight ? "initial" : "0.75rem")}; z-index: 99999; padding: 0.75rem; - bottom: 33vh; + bottom: 50%; width: 3rem; height: 3rem; background-color: var(--fo-palette-background-button); diff --git a/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx b/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx index 249285433f..b848024881 100644 --- a/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx +++ b/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx @@ -4,9 +4,10 @@ import React, { Suspense, useEffect } from "react"; import { useRecoilCallback, useRecoilValue, useSetRecoilState } from "recoil"; import styled from "styled-components"; import Group from "./Group"; -import { useModalContext } from "./hooks"; +import { useLookerHelpers, useModalContext } from "./hooks"; import { Sample2D } from "./Sample2D"; import { Sample3d } from "./Sample3d"; +import ModalNavigation from "./ModalNavigation"; const ContentColumn = styled.div` display: flex; @@ -27,6 +28,10 @@ export const ModalSample = React.memo(() => { const setIsTooltipLocked = useSetRecoilState(fos.isTooltipLocked); const setTooltipDetail = useSetRecoilState(fos.tooltipDetail); + const sidebarwidth = useRecoilValue(fos.sidebarWidth(true)); + const isSidebarVisible = useRecoilValue(fos.sidebarVisible(true)); + const { onNavigate } = useLookerHelpers(); + const tooltipEventHandler = useRecoilCallback( ({ snapshot, set }) => (e) => { @@ -72,6 +77,9 @@ export const ModalSample = React.memo(() => { return ( + + + {}}> {isGroup ? : is3D ? : } @@ -80,3 +88,13 @@ export const ModalSample = React.memo(() => { ); }); + +const ModalNavigationContainer = styled.div` + display: flex; + justify-content: space-between; + align-items: center; + width: 100%; + height: 100%; + position: absolute; + left: 0; +`; diff --git a/app/packages/core/src/components/Sidebar/Sidebar.tsx b/app/packages/core/src/components/Sidebar/Sidebar.tsx index 9395703178..bbcecdefef 100644 --- a/app/packages/core/src/components/Sidebar/Sidebar.tsx +++ b/app/packages/core/src/components/Sidebar/Sidebar.tsx @@ -733,6 +733,7 @@ const InteractiveSidebar = ({ scroll.current = target.scrollTop; down.current && animate(last.current); }} + style={modal ? { maxHeight: "calc(100% - 28px)" } : {}} > { const open = Boolean(anchorEl); const id = open ? "simple-popover" : undefined; - const theme = useTheme(); return ( theme.zIndex.tooltip }} > true, + Icon: ViewInAr, }); diff --git a/app/packages/operators/src/useCustomPanelHooks.ts b/app/packages/operators/src/useCustomPanelHooks.ts index b9ae319965..a4f41b967c 100644 --- a/app/packages/operators/src/useCustomPanelHooks.ts +++ b/app/packages/operators/src/useCustomPanelHooks.ts @@ -2,15 +2,15 @@ import { debounce, merge } from "lodash"; import { useCallback, useEffect, useMemo } from "react"; import { usePanelState, useSetCustomPanelState } from "@fiftyone/spaces"; +import { useUnboundState } from "@fiftyone/state"; import { PANEL_STATE_CHANGE_DEBOUNCE, PANEL_STATE_PATH_CHANGE_DEBOUNCE, } from "./constants"; import { executeOperator } from "./operators"; -import { useGlobalExecutionContext } from "./state"; +import { useCurrentSample, useGlobalExecutionContext } from "./state"; import usePanelEvent from "./usePanelEvent"; import { memoizedDebounce } from "./utils"; -import { useUnboundState } from "@fiftyone/state"; export interface CustomPanelProps { panelId: string; @@ -74,6 +74,7 @@ export function useCustomPanelHooks(props: CustomPanelProps): CustomPanelHooks { }); const panelSchema = panelStateLocal?.schema; const ctx = useGlobalExecutionContext(); + const currentSample = useCurrentSample(); const isLoaded: boolean = useMemo(() => { return panelStateLocal?.loaded; }, [panelStateLocal?.loaded]); @@ -119,7 +120,7 @@ export function useCustomPanelHooks(props: CustomPanelProps): CustomPanelHooks { useCtxChangePanelEvent( isLoaded, panelId, - ctx.currentSample, + currentSample, props.onChangeCurrentSample ); useCtxChangePanelEvent( diff --git a/app/packages/spaces/src/components/AddPanelButton.tsx b/app/packages/spaces/src/components/AddPanelButton.tsx index 4897c1efe5..4969fe08b0 100644 --- a/app/packages/spaces/src/components/AddPanelButton.tsx +++ b/app/packages/spaces/src/components/AddPanelButton.tsx @@ -2,8 +2,8 @@ import { IconButton, Popout, scrollable } from "@fiftyone/components"; import { PluginComponentRegistration } from "@fiftyone/plugins"; import * as fos from "@fiftyone/state"; import { Add } from "@mui/icons-material"; -import { useMemo, useRef, useState } from "react"; -import { useRecoilCallback } from "recoil"; +import { useCallback, useMemo, useRef, useState } from "react"; +import { useRecoilValue } from "recoil"; import { usePanels, useSpaceNodes } from "../hooks"; import { AddPanelButtonProps } from "../types"; import { panelsCompareFn } from "../utils/sort"; @@ -12,23 +12,16 @@ import { AddPanelButtonContainer } from "./StyledElements"; export default function AddPanelButton({ node, spaceId }: AddPanelButtonProps) { const [open, setOpen] = useState(false); - const panelsPredicate = useRecoilCallback( - ({ snapshot }) => - (panel: PluginComponentRegistration) => { - const isModalActive = snapshot - .getLoadable(fos.isModalActive) - .valueOrThrow(); - const surface = panel.panelOptions?.surfaces; - - if (isModalActive) { - return surface === "modal" || surface === "grid modal"; - } - - if (surface === "modal") return false; - - return true; - }, - [] + const isModalActive = useRecoilValue(fos.isModalActive); + const panelsPredicate = useCallback( + (panel: PluginComponentRegistration) => { + const surface = panel.panelOptions?.surfaces; + if (isModalActive) { + return surface === "modal" || surface === "grid modal"; + } + return surface !== "modal"; + }, + [isModalActive] ); const panels = usePanels(panelsPredicate); const spaceNodes = useSpaceNodes(spaceId); diff --git a/app/packages/spaces/src/components/StyledElements.tsx b/app/packages/spaces/src/components/StyledElements.tsx index 057de01085..20e5bb968e 100644 --- a/app/packages/spaces/src/components/StyledElements.tsx +++ b/app/packages/spaces/src/components/StyledElements.tsx @@ -12,20 +12,15 @@ export const PanelContainer = styled.div` overflow: hidden; `; -export const PanelTabs = styled.div<{ $isModal?: boolean }>` +export const PanelTabs = styled.div` display: flex; background: var(--fo-palette-background-header); padding-bottom: 0px; - position: ${(props) => (props.$isModal ? "absolute" : "initial")}; - height: ${(props) => (props.$isModal ? "2em" : "initial")}; - z-index: 100001; - width: 100%; `; -export const StyledPanel = styled.div<{ $isModalPanel?: boolean }>` +export const StyledPanel = styled.div` width: 100%; - height: ${(props) => (props.$isModalPanel ? "100%" : "calc(100% - 28px)")}; - padding: ${(props) => (props.$isModalPanel ? "2.2em 5px 0 5px" : "initial")}; + height: calc(100% - 28px); overflow: auto; background: var(--fo-palette-background-mediaSpace); `; From fca64f68cf5b19df4f6182866f15a9843c25f8bc Mon Sep 17 00:00:00 2001 From: imanjra Date: Wed, 11 Sep 2024 01:24:13 -0400 Subject: [PATCH 2/2] use only local state atom for modal spaces --- .../operators/src/built-in-operators.ts | 5 +- app/packages/spaces/src/components/Panel.tsx | 13 +++-- app/packages/spaces/src/contexts.ts | 4 +- app/packages/spaces/src/hooks.ts | 50 ++++++++++++++----- app/packages/spaces/src/state.ts | 18 ++++--- app/packages/spaces/src/types.ts | 1 + 6 files changed, 64 insertions(+), 27 deletions(-) diff --git a/app/packages/operators/src/built-in-operators.ts b/app/packages/operators/src/built-in-operators.ts index 81bbd15ed1..e99f68ce67 100644 --- a/app/packages/operators/src/built-in-operators.ts +++ b/app/packages/operators/src/built-in-operators.ts @@ -1,7 +1,6 @@ import { Layout, SpaceNode, - usePanelState, usePanelTitle, usePanels, useSetPanelStateById, @@ -12,6 +11,7 @@ import * as fos from "@fiftyone/state"; import * as types from "./types"; import { useTrackEvent } from "@fiftyone/analytics"; +import { setPathUserUnchanged } from "@fiftyone/core/src/plugins/SchemaIO/hooks"; import { LOAD_WORKSPACE_OPERATOR } from "@fiftyone/spaces/src/components/Workspaces/constants"; import { toSlug } from "@fiftyone/utilities"; import copyToClipboard from "copy-to-clipboard"; @@ -31,7 +31,6 @@ import { } from "./operators"; import { useShowOperatorIO } from "./state"; import usePanelEvent from "./usePanelEvent"; -import { setPathUserUnchanged } from "@fiftyone/core/src/plugins/SchemaIO/hooks"; // // BUILT-IN OPERATORS @@ -1029,8 +1028,6 @@ class PromptUserForOperation extends Operator { return new types.Property(inputs); } useHooks(ctx: ExecutionContext): {} { - const panelId = ctx.getCurrentPanelId(); - const [panelState] = usePanelState(panelId); const triggerEvent = usePanelEvent(); return { triggerEvent }; } diff --git a/app/packages/spaces/src/components/Panel.tsx b/app/packages/spaces/src/components/Panel.tsx index 517f1770e6..3b561dd5b9 100644 --- a/app/packages/spaces/src/components/Panel.tsx +++ b/app/packages/spaces/src/components/Panel.tsx @@ -1,11 +1,12 @@ import { CenteredStack, scrollable } from "@fiftyone/components"; import * as fos from "@fiftyone/state"; -import React, { useMemo } from "react"; -import { useRecoilValue } from "recoil"; +import React, { useEffect, useMemo } from "react"; +import { useRecoilValue, useSetRecoilState } from "recoil"; import { PANEL_LOADING_TIMEOUT } from "../constants"; import { PanelContext } from "../contexts"; import { useReactivePanel } from "../hooks"; import SpaceNode from "../SpaceNode"; +import { panelIdToScopeAtom } from "../state"; import { PanelProps } from "../types"; import PanelNotFound from "./PanelNotFound"; import PanelSkeleton from "./PanelSkeleton"; @@ -37,6 +38,12 @@ function Panel(props: PanelProps) { const panel = useReactivePanel(panelName); const dimensions = fos.useDimensions(); const pending = fos.useTimeout(PANEL_LOADING_TIMEOUT); + const setPanelIdToScope = useSetRecoilState(panelIdToScopeAtom); + const scope = isModalPanel ? "modal" : "grid"; + + useEffect(() => { + setPanelIdToScope((ids) => ({ ...ids, [node.id]: scope })); + }, [scope, setPanelIdToScope, node.id]); const panelContentTestId = `panel-content-${panelName}`; if (!panel) { @@ -63,7 +70,7 @@ function Panel(props: PanelProps) { className={scrollable} ref={dimensions.ref} > - + {isModalPanel ? ( ({}); +export const PanelContext = createContext({}); + +type PanelContextType = { node?: SpaceNode; scope?: string }; diff --git a/app/packages/spaces/src/hooks.ts b/app/packages/spaces/src/hooks.ts index e05e4cfeed..3d9eaafb4f 100644 --- a/app/packages/spaces/src/hooks.ts +++ b/app/packages/spaces/src/hooks.ts @@ -23,6 +23,7 @@ import { import SpaceTree from "./SpaceTree"; import { PanelContext } from "./contexts"; import { + panelIdToScopeAtom, panelStatePartialSelector, panelStateSelector, panelTitlesState, @@ -245,27 +246,35 @@ export function usePanelContext() { export function usePanelState( defaultState?: T, id?: string, - local?: boolean + local?: boolean, + scope?: string ) { + const panelScope = useScope(scope); const panelContext = usePanelContext(); const panelId = id || (panelContext?.node?.id as string); const [state, setState] = useRecoilState( - panelStateSelector({ panelId, local }) + panelStateSelector({ panelId, local, scope: panelScope }) ); const computedState = state || defaultState; return [computedState, setState]; } -export function useSetPanelStateById(local?: boolean) { +export function useSetPanelStateById(local?: boolean, scope?: string) { + const panelScope = useScope(scope); return useRecoilCallback( ({ set, snapshot }) => async (panelId: string, fn: (state: any) => any) => { + const panelIdToScope = await snapshot.getPromise(panelIdToScopeAtom); + const computedScope = panelScope || panelIdToScope?.[panelId]; const panelState = await snapshot.getPromise( - panelStateSelector({ panelId, local }) + panelStateSelector({ panelId, local, scope: computedScope }) ); const updatedValue = fn(panelState); - set(panelStateSelector({ panelId, local }), updatedValue); + set( + panelStateSelector({ panelId, local, scope: computedScope }), + updatedValue + ); }, [] ); @@ -293,15 +302,17 @@ export function useSetCustomPanelState(local?: boolean) { */ export function usePanelStateCallback( callback: (panelState: T) => void, - local?: boolean + local?: boolean, + scope?: string ) { + const panelScope = useScope(scope); const panelContext = usePanelContext(); const panelId = panelContext?.node?.id as string; return useRecoilCallback( ({ snapshot }) => async () => { const panelState = await snapshot.getPromise( - panelStateSelector({ panelId, local }) + panelStateSelector({ panelId, local, scope: panelScope }) ); callback(panelState); }, @@ -311,13 +322,15 @@ export function usePanelStateCallback( export function usePanelStateByIdCallback( callback: (panelId: string, panelState: T, args: any[]) => void, - local?: boolean + local?: boolean, + scope?: string ) { + const panelScope = useScope(scope); return useRecoilCallback( ({ snapshot }) => async (panelId: string, ...args) => { const panelState = await snapshot.getPromise( - panelStateSelector({ panelId, local }) + panelStateSelector({ panelId, local, scope: panelScope }) ); callback(panelId, panelState, args as any[]); }, @@ -330,14 +343,17 @@ export function usePanelStateByIdCallback( * @returns a state resolver function which return promise that resolves to the * current state of a panel */ -export function usePanelStateLazy(local?: boolean) { +export function usePanelStateLazy(local?: boolean, scope?: string) { + const panelScope = useScope(scope); const panelContext = usePanelContext(); const panelId = panelContext?.node?.id as string; const resolvePanelState = useRecoilCallback( ({ snapshot }) => async () => - snapshot.getPromise(panelStateSelector({ panelId, local })) + snapshot.getPromise( + panelStateSelector({ panelId, local, scope: panelScope }) + ) ); return () => resolvePanelState(); @@ -352,12 +368,14 @@ export function usePanelStateLazy(local?: boolean) { export function usePanelStatePartial( key: string, defaultState: T, - local?: boolean + local?: boolean, + scope?: string ) { + const panelScope = useScope(scope); const panelContext = usePanelContext(); const panelId = panelContext?.node?.id as string; const [state, setState] = useRecoilState( - panelStatePartialSelector({ panelId, key, local }) + panelStatePartialSelector({ panelId, key, local, scope: panelScope }) ); const computedState = useComputedState(state, defaultState); return [computedState, setState]; @@ -436,3 +454,9 @@ export function usePanelCloseEffect(panelId?: string) { } }; } + +function useScope(scope?: string) { + const panelContext = usePanelContext(); + if (typeof scope === "string") return scope; + return panelContext?.scope; +} diff --git a/app/packages/spaces/src/state.ts b/app/packages/spaces/src/state.ts index 3214aaa0e9..7cb54b6e69 100644 --- a/app/packages/spaces/src/state.ts +++ b/app/packages/spaces/src/state.ts @@ -58,15 +58,15 @@ export const panelStateSelector = selectorFamily({ get: (params: PanelStateParameter) => ({ get }) => { - const { panelId, local } = params; - const stateAtom = getStateAtom(local); + const { panelId, local, scope } = params; + const stateAtom = getStateAtom(local, scope); return get(stateAtom).get(panelId); }, set: (params: PanelStateParameter) => ({ get, set }, newValue) => { - const { panelId, local } = params; - const stateAtom = getStateAtom(local); + const { panelId, local, scope } = params; + const stateAtom = getStateAtom(local, scope); const newState = new Map(get(stateAtom)); newState.set(panelId, newValue); set(stateAtom, newState); @@ -125,6 +125,12 @@ export const savedWorkspacesAtom = atom({ }, }); -function getStateAtom(local?: boolean) { - return local ? panelsLocalStateAtom : panelsStateAtom; +export const panelIdToScopeAtom = atom({ + key: "panelIdToScopeAtom", + default: {}, +}); + +function getStateAtom(local?: boolean, scope?: string) { + const nonGridScope = scope !== "grid"; + return local || nonGridScope ? panelsLocalStateAtom : panelsStateAtom; } diff --git a/app/packages/spaces/src/types.ts b/app/packages/spaces/src/types.ts index b3dea93167..2510410a75 100644 --- a/app/packages/spaces/src/types.ts +++ b/app/packages/spaces/src/types.ts @@ -65,6 +65,7 @@ export type SpaceProps = { export type PanelStateParameter = { panelId: string; local?: boolean; + scope?: string; }; export type PanelStatePartialParameter = PanelStateParameter & {