Merge pull request #4078 from voxel51/merge/v0.23.5-to-main

Merge/v0.23.5 to main

app/packages/core/src/components/Actions/ActionsRow.tsx

@@ -168,9 +168,7 @@ const Tag = ({
   const tagging = useRecoilValue(fos.anyTagging);
   const ref = useRef<HTMLDivElement>(null);
   useOutsideClick(ref, () => open && setOpen(false));
-  const lightning = useRecoilValue(fos.lightning);
-  const unlocked = fos.useLightingUnlocked();
-  const disabled = tagging || (lightning && !modal && !unlocked);
+  const disabled = tagging;
 
   lookerRef &&
     useEventHandler(lookerRef.current, "play", () => {

app/packages/core/src/components/Actions/Options.tsx

@@ -193,6 +193,10 @@ const DynamicGroupsViewMode = ({ modal }: { modal: boolean }) => {
     return options;
   }, [isOrderedDynamicGroup, hasGroupSlices]);
 
+  if (!modal && !isOrderedDynamicGroup) {
+    return null;
+  }
+
   return (
     <>
       <PopoutSectionTitle>Dynamic Groups Navigation</PopoutSectionTitle>

app/packages/core/src/components/Common/Checkbox.tsx

@@ -2,7 +2,7 @@ import { LoadingDots, useTheme } from "@fiftyone/components";
 import { Checkbox as MaterialCheckbox } from "@mui/material";
 import { animated } from "@react-spring/web";
 import React, { useMemo } from "react";
-import { RecoilValueReadOnly, constSelector } from "recoil";
+import { constSelector, RecoilValueReadOnly } from "recoil";
 import styled from "styled-components";
 import { prettify } from "../../utils/generic";
 import { ItemAction } from "../Actions/ItemAction";
@@ -17,7 +17,7 @@ interface CheckboxProps<T> {
   loading?: boolean;
   value: boolean;
   setValue?: (value: boolean) => void;
-  count?: number;
+  count?: number | RecoilValueReadOnly<number>;
   subcountAtom?: RecoilValueReadOnly<number>;
   disabled?: boolean;
   muted?: boolean;
@@ -56,7 +56,7 @@ function Checkbox<T>({
   const [text, coloring] = getValueString(formatter ? formatter(name) : name);
 
   const countAtom = useMemo(
-    () => (typeof count === "number" ? constSelector(count) : null),
+    () => (typeof count === "number" ? constSelector(count) : count ?? null),
     [count]
   );
 

app/packages/core/src/components/Filters/StringFilter/Checkboxes.tsx

@@ -1,6 +1,6 @@
 import { LoadingDots } from "@fiftyone/components";
 import * as fos from "@fiftyone/state";
-import React, { MutableRefObject } from "react";
+import React from "react";
 import {
   RecoilState,
   selectorFamily,
@@ -14,6 +14,7 @@ import { isBooleanField, isInKeypointsField } from "../state";
 import { CHECKBOX_LIMIT, nullSort } from "../utils";
 import Reset from "./Reset";
 import { Result } from "./Result";
+import { pathSearchCount } from "./state";
 import { showSearchSelector } from "./useSelected";
 
 interface CheckboxesProps {
@@ -23,7 +24,6 @@ interface CheckboxesProps {
   isMatchingAtom: RecoilState<boolean>;
   modal: boolean;
   path: string;
-  selectedMap: MutableRefObject<Map<string | null, number | null>>;
 }
 
 const isSkeleton = selectorFamily({
@@ -86,13 +86,11 @@ const useValues = ({
   path,
   results,
   selected,
-  selectedMap,
 }: {
   modal: boolean;
   path: string;
   results: Result[] | null;
   selected: (string | null)[];
-  selectedMap: Map<string | null, number | null>;
 }) => {
   const name = path.split(".").slice(-1)[0];
   const unlocked = fos.useLightingUnlocked();
@@ -106,7 +104,7 @@ const useValues = ({
 
   let allValues = selected.map((value) => ({
     value,
-    count: hasCount ? counts.get(value) || selectedMap.get(value) || 0 : null,
+    count: hasCount ? counts.get(value) ?? null : null,
     loading: unlocked && loading,
   }));
   const objectId = useRecoilValue(fos.isObjectIdField(path));
@@ -149,7 +147,6 @@ const Checkboxes = ({
   isMatchingAtom,
   modal,
   path,
-  selectedMap,
 }: CheckboxesProps) => {
   const [selected, setSelected] = useRecoilState(selectedAtom);
   const color = useRecoilValue(fos.pathColor(path));
@@ -160,7 +157,6 @@ const Checkboxes = ({
     path,
     results,
     selected,
-    selectedMap: selectedMap.current,
   });
 
   const show = useRecoilValue(showSearchSelector({ modal, path }));
@@ -191,8 +187,9 @@ const Checkboxes = ({
             loading={loading}
             count={
               typeof count !== "number" || !isFilterMode || keypoints
-                ? undefined
-                : selectedMap.current.get(value) ?? count
+                ? // only string and id fields use pathSearchCount
+                  pathSearchCount({ modal, path, value: value as string })
+                : count
             }
             setValue={(checked: boolean) => {
               if (checked) {

app/packages/core/src/components/Filters/StringFilter/StringFilter.tsx

@@ -69,7 +69,7 @@ const StringFilter = ({
     path,
     resultsAtom
   );
-  const { onSelect, selectedMap } = useOnSelect(modal, path, selectedAtom);
+  const onSelect = useOnSelect(modal, path, selectedAtom);
   const skeleton =
     useRecoilValue(isInKeypointsField(path)) && name === "keypoints";
   const theme = useTheme();
@@ -127,7 +127,6 @@ const StringFilter = ({
           excludeAtom={excludeAtom}
           isMatchingAtom={isMatchingAtom}
           modal={modal}
-          selectedMap={selectedMap}
         />
       </StringFilterContainer>
     </NamedStringFilterContainer>

app/packages/core/src/components/Filters/StringFilter/state.ts

@@ -1,4 +1,5 @@
 import * as fos from "@fiftyone/state";
+import { isMatchingAtom, stringExcludeAtom } from "@fiftyone/state";
 import { getFetchFunction } from "@fiftyone/utilities";
 import { atomFamily, selectorFamily } from "recoil";
 import { labelTagsCount } from "../../Sidebar/Entries/EntryCounts";
@@ -13,18 +14,49 @@ export const stringSearch = atomFamily<
   default: "",
 });
 
+const pathSearchFilters = selectorFamily({
+  key: "pathSearchFilters",
+  get:
+    ({ modal, path }: { modal: boolean; path: string }) =>
+    ({ get }) => {
+      const filters = { ...get(modal ? fos.modalFilters : fos.filters) };
+
+      // omit the path being searched, but include coinciding filters
+      delete filters[path];
+
+      return filters;
+    },
+});
+
+export const pathSearchCount = selectorFamily({
+  key: "pathSearchCount",
+  get:
+    ({ modal, path, value }: { modal: boolean; path: string; value: string }) =>
+    ({ get }) => {
+      return (
+        get(
+          stringSearchResults({
+            modal,
+            path,
+            filter: { path, value },
+          })
+        )?.values?.[0].count || 0
+      );
+    },
+});
+
 export const stringSearchResults = selectorFamily<
   {
     values?: Result[];
     count?: number;
   },
-  { path: string; modal: boolean }
+  { path: string; modal: boolean; filter?: { path: string; value: string } }
 >({
   key: "stringSearchResults",
   get:
-    ({ path, modal }) =>
+    ({ path, modal, filter }) =>
     async ({ get }) => {
-      const search = get(stringSearch({ modal, path }));
+      const search = filter ? "" : get(stringSearch({ modal, path }));
       const sorting = get(fos.sortFilterResults(modal));
       const mixed = get(fos.groupStatistics(modal)) === "group";
       const selected = get(fos.stringSelectedValuesAtom({ path, modal }));
@@ -61,8 +93,16 @@ export const stringSearchResults = selectorFamily<
           view: get(fos.view),
           path,
           search,
-          selected,
-          filters: !modal ? get(fos.lightningFilters) : null,
+          selected: filter ? [] : selected,
+          filters: filter
+            ? {
+                [filter.path]: {
+                  exclude: get(stringExcludeAtom({ path, modal })),
+                  isMatching: get(isMatchingAtom({ path, modal })),
+                  values: [filter.value],
+                },
+              }
+            : get(pathSearchFilters({ modal, path })),
           group_id: modal ? get(fos.groupId) || null : null,
           mixed,
           slice: get(fos.groupSlice),

app/packages/core/src/components/Filters/StringFilter/useOnSelect.ts

@@ -1,7 +1,6 @@
 import { SelectorValidationError } from "@fiftyone/components";
 import { isObjectIdField, snackbarErrors } from "@fiftyone/state";
 import { isObjectIdString } from "@fiftyone/utilities";
-import { useRef } from "react";
 import { RecoilState, useRecoilCallback } from "recoil";
 import { Result } from "./Result";
 
@@ -10,31 +9,25 @@ export default function (
   path: string,
   selectedAtom: RecoilState<(string | null)[]>
 ) {
-  const selectedMap = useRef<Map<string | null, number | null>>(new Map());
-  return {
-    onSelect: useRecoilCallback(
-      ({ snapshot, set }) =>
-        async (value: string | null, d?: Result) => {
-          const isObjectId = await snapshot.getPromise(isObjectIdField(path));
-          if (isObjectId && (value === null || !isObjectIdString(value))) {
-            set(snackbarErrors, [
-              `${value} is not a 24 character hexadecimal string`,
-            ]);
-            throw new SelectorValidationError();
-          }
+  return useRecoilCallback(
+    ({ snapshot, set }) =>
+      async (value: string | null, d?: Result) => {
+        const isObjectId = await snapshot.getPromise(isObjectIdField(path));
+        if (isObjectId && (value === null || !isObjectIdString(value))) {
+          set(snackbarErrors, [
+            `${value} is not a 24 character hexadecimal string`,
+          ]);
+          throw new SelectorValidationError();
+        }
 
-          const selected = new Set(await snapshot.getPromise(selectedAtom));
-          if (d?.value === null) {
-            value = null;
-          }
-          selectedMap.current.set(value, d?.count || null);
-          selected.add(value);
-          set(selectedAtom, [...selected].sort());
-
-          return "";
-        },
-      [modal, path, selectedAtom, selectedMap]
-    ),
-    selectedMap,
-  };
+        const selected = new Set(await snapshot.getPromise(selectedAtom));
+        if (d?.value === null) {
+          value = null;
+        }
+        selected.add(value);
+        set(selectedAtom, [...selected].sort());
+        return "";
+      },
+    [modal, path, selectedAtom]
+  );
 }

app/packages/core/src/components/Modal/Group/DynamicGroup/index.tsx

@@ -1,6 +1,6 @@
 import * as fos from "@fiftyone/state";
 import React, { useEffect } from "react";
-import { useRecoilValue, useSetRecoilState } from "recoil";
+import { useRecoilState, useRecoilValue, useSetRecoilState } from "recoil";
 import { NestedGroup } from "./NestedGroup";
 import { NonNestedDynamicGroup } from "./NonNestedGroup";
 import { useDynamicGroupSamples } from "./useDynamicGroupSamples";
@@ -11,6 +11,11 @@ export const DynamicGroup = () => {
   const { queryRef } = useDynamicGroupSamples();
 
   const shouldRenderImaVid = useRecoilValue(fos.shouldRenderImaVidLooker);
+  const [dynamicGroupsViewMode, setDynamicGroupsViewMode] = useRecoilState(
+    fos.dynamicGroupsViewMode
+  );
+  const isOrderedDynamicGroup = useRecoilValue(fos.isOrderedDynamicGroup);
+
   const setDynamicGroupCurrentElementIndex = useSetRecoilState(
     fos.dynamicGroupCurrentElementIndex
   );
@@ -26,6 +31,14 @@ export const DynamicGroup = () => {
     }
   }, [shouldRenderImaVid, imaVidIndex, setDynamicGroupCurrentElementIndex]);
 
+  useEffect(() => {
+    // if dynamic group view mode is video but dynamic group is not ordered,
+    // we want to set view mode back to pagination (default)
+    if (dynamicGroupsViewMode === "video" && !isOrderedDynamicGroup) {
+      setDynamicGroupsViewMode("pagination");
+    }
+  }, [dynamicGroupsViewMode, isOrderedDynamicGroup]);
+
   return (
     <>
       {hasGroupSlices ? (

app/packages/core/src/components/Sidebar/Entries/PathValueEntry.tsx

@@ -296,7 +296,7 @@ const SlicesLoadable = ({ path }: { path: string }) => {
           >
             <div style={{ color: theme.text.secondary }}>{slice}</div>
             <div
-              data-cy={`sidebar-entry-${path}`}
+              data-cy={`sidebar-entry-${slice}-${path}`}
               style={{
                 ...add,
                 flex: 1,
@@ -333,16 +333,16 @@ const useSlicesData = <T extends unknown>(path: string) => {
 
   const data = { ...loadable.contents };
 
-  let field = useRecoilValue(fos.field(keys[0]));
+  const target = useRecoilValue(fos.field(keys[0]));
   slices.forEach((slice) => {
     let sliceData = data[slice].sample;
+    let field = target;
 
     for (let index = 0; index < keys.length; index++) {
       if (!sliceData) {
         break;
       }
       const key = keys[index];
-
       sliceData = sliceData[field?.dbField || key];
 
       if (keys[index + 1]) {

app/packages/core/src/components/Sidebar/Sidebar.tsx

@@ -382,7 +382,7 @@ const SidebarColumn = styled.div`
 const Container = animated(styled.div`
   position: relative;
   min-height: 100%;
-  margin: 0 0.25rem 0 1rem;
+  margin: 0 1rem;
 
   & > div {
     position: absolute;

app/packages/core/src/components/Sidebar/ViewSelection/ViewDialog.tsx

@@ -194,6 +194,8 @@ export default function ViewDialog(props: Props) {
 
   return (
     <Dialog
+      // might have to disablePortal or disableEnforceFocus if we run into issues
+      // see https://github.com/mui/material-ui/issues/10341
       open={isOpen}
       onClose={() => {
         setIsOpen(false);

app/packages/looker-3d/src/action-bar/SliceSelector.tsx

@@ -43,7 +43,10 @@ export const SliceSelector = () => {
 
   return (
     <>
-      <ActionItem title="Select point clouds">
+      <ActionItem
+        data-cy={"looker3d-select-slices"}
+        title="Select point clouds"
+      >
         <div onClick={handleActionClick}>{activeSlicesLabel}</div>
       </ActionItem>
 
@@ -70,7 +73,7 @@ const PcdsSelector = () => {
   return (
     <ActionPopOver>
       <PopoutSectionTitle>Select point clouds</PopoutSectionTitle>
-      <div>
+      <div data-cy={"looker3d-slice-checkboxes"}>
         {allPcdSlices.map((slice) => {
           return (
             <Checkbox

app/packages/looker/src/elements/util.ts

@@ -83,7 +83,7 @@ const stringifyNumber = function (
   pad: boolean = false
 ): string {
   let str = "";
-  if (pad && number <= 10) {
+  if (pad && number < 10) {
     str += "0" + number;
   } else if (number === 0) {
     str = "0";