diff --git a/app/packages/components/src/components/Selector/Selector.tsx b/app/packages/components/src/components/Selector/Selector.tsx index a009e9400e..6d22d8fd41 100644 --- a/app/packages/components/src/components/Selector/Selector.tsx +++ b/app/packages/components/src/components/Selector/Selector.tsx @@ -13,6 +13,8 @@ import LoadingDots from "../Loading/LoadingDots"; import SearchResults, { UseSearch } from "./SearchResults"; import style from "./Selector.module.css"; +export class SelectorValidationError extends Error {} + export interface SelectorProps { id?: string; value?: string; @@ -69,11 +71,18 @@ function Selector(props: SelectorProps) { ? valuesRef.current[active] : valuesRef.current.find((v) => toKey(v) === search); - const result = await onSelect(value ? toKey(value) : search, value); - if (result !== undefined) { - local.current = result; + try { + const result = await onSelect(value ? toKey(value) : search, value); + if (result !== undefined) { + local.current = result; + } + setEditing(false); + } catch (error) { + if (error instanceof SelectorValidationError) { + return; + } + throw error; } - setEditing(false); }; }, [active, onSelect, toKey, valuesRef]); diff --git a/app/packages/components/src/components/Selector/index.ts b/app/packages/components/src/components/Selector/index.ts index f7790e78a8..ce2bb2084d 100644 --- a/app/packages/components/src/components/Selector/index.ts +++ b/app/packages/components/src/components/Selector/index.ts @@ -1,2 +1,2 @@ -export { default } from "./Selector"; -export type { UseSearch } from "./Selector"; +export type { UseSearch } from "./SearchResults"; +export { SelectorValidationError, default } from "./Selector"; diff --git a/app/packages/components/src/components/index.ts b/app/packages/components/src/components/index.ts index fe3725525b..4585d553af 100644 --- a/app/packages/components/src/components/index.ts +++ b/app/packages/components/src/components/index.ts @@ -19,7 +19,7 @@ export { default as Popout, PopoutDiv } from "./Popout"; export { default as PopoutSectionTitle } from "./PopoutSectionTitle"; export * from "./Selection"; export { default as Selection } from "./Selection"; -export { default as Selector } from "./Selector"; +export { default as Selector, SelectorValidationError } from "./Selector"; export type { UseSearch } from "./Selector"; export { default as TabOption } from "./TabOption"; export { default as ThemeProvider, useTheme } from "./ThemeProvider"; diff --git a/app/packages/core/src/components/Actions/Options.tsx b/app/packages/core/src/components/Actions/Options.tsx index 513919a1eb..199c00d457 100644 --- a/app/packages/core/src/components/Actions/Options.tsx +++ b/app/packages/core/src/components/Actions/Options.tsx @@ -235,6 +235,7 @@ const Lightning = () => { options={["disable", "enable"].map((value) => ({ text: value, title: value, + dataCy: `lightning-mode-${value}`, onClick: () => setThreshold(value === "disable" ? null : config ?? count), }))} diff --git a/app/packages/core/src/components/Filters/StringFilter/StringFilter.tsx b/app/packages/core/src/components/Filters/StringFilter/StringFilter.tsx index eeabcd163a..b387e517e1 100644 --- a/app/packages/core/src/components/Filters/StringFilter/StringFilter.tsx +++ b/app/packages/core/src/components/Filters/StringFilter/StringFilter.tsx @@ -106,6 +106,7 @@ const StringFilter = ({ placeholder={`+ ${ isFilterMode ? "filter" : "set visibility" } by ${name}`} + cy={`sidebar-search-${path}`} component={ResultComponent} onSelect={onSelect} inputStyle={{ diff --git a/app/packages/core/src/components/Filters/StringFilter/useOnSelect.ts b/app/packages/core/src/components/Filters/StringFilter/useOnSelect.ts index fed033e55b..96703805af 100644 --- a/app/packages/core/src/components/Filters/StringFilter/useOnSelect.ts +++ b/app/packages/core/src/components/Filters/StringFilter/useOnSelect.ts @@ -1,3 +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"; @@ -12,6 +15,14 @@ export default function ( 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(); + } + const selected = new Set(await snapshot.getPromise(selectedAtom)); if (d?.value === null) { value = null; @@ -19,6 +30,7 @@ export default function ( selectedMap.current.set(value, d?.count || null); selected.add(value); set(selectedAtom, [...selected].sort()); + return ""; }, [modal, path, selectedAtom, selectedMap] diff --git a/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx b/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx index 725418a50f..79b9ac9f4b 100644 --- a/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx +++ b/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx @@ -27,7 +27,7 @@ export default ({ return ( } placement="top-center"> @@ -36,7 +36,7 @@ export default ({ return ( { } return null; }; + +export const cleanCSV = (values: string) => { + return ( + values + // replace spaces with a single space (to allow search by words with spaces) + .replace(/[\s\'\"\[\]]+/g, " ") + // replace comma followed by trailing spaces with a single comma + .replace(/,\s*/g, ",") + // remove trailing spaces + .replace(/[ \t]+$/, "") + ); +}; + +export const getArray = (value: string | unknown[]): unknown[] => { + if (typeof value === "string") { + try { + return JSON.parse(value); + } catch { + return value.replace(/[\s]/g, "").split(","); + } + } + + return value; +}; diff --git a/app/packages/core/src/components/ViewBar/ViewStage/viewStageParameterMachine.ts b/app/packages/core/src/components/ViewBar/ViewStage/viewStageParameterMachine.ts index a6ef668c4a..a0a31bb4a5 100644 --- a/app/packages/core/src/components/ViewBar/ViewStage/viewStageParameterMachine.ts +++ b/app/packages/core/src/components/ViewBar/ViewStage/viewStageParameterMachine.ts @@ -1,7 +1,8 @@ import { v4 as uuid } from "uuid"; import { actions, Machine, sendParent } from "xstate"; -import { computeBestMatchString, getMatch } from "./utils"; +import { isObjectIdString } from "@fiftyone/utilities"; +import { cleanCSV, computeBestMatchString, getArray, getMatch } from "./utils"; const { assign } = actions; @@ -85,7 +86,7 @@ export const PARSER = { castFrom: (value) => value, castTo: (value) => value, parse: (value) => value, - validate: (value) => /[0-9A-Fa-f]{24}/g.test(value), + validate: (value) => isObjectIdString(value), }, int: { castFrom: (value) => String(value).replace(/\B(?=(\d{3})+(?!\d))/g, ","), @@ -113,66 +114,25 @@ export const PARSER = { "list": { castFrom: (value) => value.join(","), castTo: (value) => value.split(","), - parse: (value) => value.replace(/[\s\'\"\[\]]/g, ""), + parse: (value) => cleanCSV(value), validate: (value, fields) => { - const stripped = value.replace(/[\s]/g, ""); - let array = null; - try { - array = JSON.parse(stripped); - } catch { - array = stripped.split(","); - } - return ( - typeof value !== "string" && - Array.isArray(array) && - array.every((e) => PARSER.field.validate(e, fields)) - ); + return getArray(value).every((e) => PARSER.field.validate(e, fields)); }, }, "list": { castFrom: (value) => value.join(","), castTo: (value) => value.split(","), - parse: (value) => value.replace(/[\s\'\"\[\]]/g, ""), + parse: (value) => cleanCSV(value), validate: (value) => { - const stripped = value.replace(/[\s]/g, ""); - let array = null; - try { - array = JSON.parse(stripped); - } catch { - array = stripped.split(","); - } - return ( - typeof value !== "string" && - Array.isArray(array) && - array.every((e) => PARSER.id.validate(e)) - ); + return getArray(value).every((e) => PARSER.id.validate(e)); }, }, "list": { castFrom: (value) => value.join(","), castTo: (value) => value.split(","), - parse: (value) => - value - // replace spaces with a single space (to allow search by words with spaces) - .replace(/[\s\'\"\[\]]+/g, " ") - // replace comma followed by trailing spaces with a single comma - .replace(/,\s*/g, ",") - // remove trailing spaces - .replace(/[ \t]+$/, ""), + parse: (value) => cleanCSV(value), validate: (value) => { - const stripped = value.replace(/[\s]/g, ""); - let array = null; - try { - array = JSON.parse(stripped); - } catch { - // ex: exclude_fields('a, b'). 'a,b' => ['a', 'b'] - array = stripped.split(","); - } - return ( - typeof array !== "string" && - Array.isArray(array) && - array.every((e) => PARSER.str.validate(e)) - ); + return getArray(value).every((e) => PARSER.str.validate(e)); }, }, NoneType: { diff --git a/app/packages/state/src/recoil/lightning.ts b/app/packages/state/src/recoil/lightning.ts index ba9efa7ce5..13ec462f40 100644 --- a/app/packages/state/src/recoil/lightning.ts +++ b/app/packages/state/src/recoil/lightning.ts @@ -103,6 +103,8 @@ const indexesByPath = selector({ ) => { const projection = frames ? framesProjection : samplesProjection; + fields = fields.map((field) => get(schemaAtoms.dbPath(field))); + if (field === "$**") { if (!projection) { return fields; @@ -143,8 +145,7 @@ export const pathIndex = selectorFamily({ (path: string) => ({ get }) => { const indexes = get(indexesByPath); - - return indexes.has(path); + return indexes.has(get(schemaAtoms.dbPath(path))); }, }); diff --git a/app/packages/utilities/src/type-check.ts b/app/packages/utilities/src/type-check.ts index 5fee55ea31..b818f64cad 100644 --- a/app/packages/utilities/src/type-check.ts +++ b/app/packages/utilities/src/type-check.ts @@ -10,6 +10,14 @@ export function isPrimitiveType(type: string) { return PRIMITIVE_TYPES.includes(type); } +export function isHex(value: string) { + return /[0-9a-f]{24}/g.test(value); +} + +export function isObjectIdString(value: string) { + return isHex(value) && value.length === 24; +} + export type NumberKeyObjectType = { [key: string]: V; }; diff --git a/e2e-pw/src/oss/poms/action-row/display-options.ts b/e2e-pw/src/oss/poms/action-row/display-options.ts index 11c3ec6fb9..3f229e2df7 100644 --- a/e2e-pw/src/oss/poms/action-row/display-options.ts +++ b/e2e-pw/src/oss/poms/action-row/display-options.ts @@ -3,6 +3,7 @@ import { Page } from "src/oss/fixtures"; type SidebarStatisticsMode = "slice" | "group"; type SidebarMode = "fast" | "best" | "all"; type SidebarSortMode = "count" | "value"; +type LightningMode = "enable" | "disable"; export class DisplayOptionsPom { readonly page: Page; @@ -11,6 +12,11 @@ export class DisplayOptionsPom { this.page = page; } + async setLightningMode(mode: LightningMode) { + const selector = this.page.getByTestId(`lightning-mode-${mode}`); + return selector.click(); + } + async setSidebarStatisticsMode(mode: SidebarStatisticsMode) { const selector = this.page.getByTestId( `tab-option-View ${mode} sidebar statistics` diff --git a/e2e-pw/src/oss/poms/sidebar.ts b/e2e-pw/src/oss/poms/sidebar.ts index 279fd363b8..fcb8f74dd1 100644 --- a/e2e-pw/src/oss/poms/sidebar.ts +++ b/e2e-pw/src/oss/poms/sidebar.ts @@ -28,6 +28,16 @@ export class SidebarPom { .nth(1); } + fieldContainer(fieldName: string) { + return this.sidebar.getByTestId(`sidebar-field-container-${fieldName}`); + } + + fieldArrow(fieldName: string, enabled: boolean) { + return this.fieldContainer(fieldName).getByTestId( + `sidebar-field-arrow-${enabled ? "enabled" : "disabled"}-${fieldName}` + ); + } + sidebarEntryDraggableArea(fieldName: string) { return this.sidebar .getByTestId(`sidebar-entry-draggable-${fieldName}`) @@ -58,7 +68,9 @@ export class SidebarPom { } async clickFieldDropdown(field: string) { - const selector = this.sidebar.getByTestId(`sidebar-field-arrow-${field}`); + const selector = this.sidebar.getByTestId( + `sidebar-field-arrow-enabled-${field}` + ); return selector.click(); } @@ -92,6 +104,12 @@ export class SidebarPom { await selectionDiv.click({ force: true }); } + async applySearch(field: string, search: string) { + const input = this.sidebar.getByTestId(`selector-sidebar-search-${field}`); + await input.fill(search); + await input.press("Enter"); + } + // apply a filter to a field async applyLabelFromList( field: string, @@ -137,9 +155,29 @@ class SidebarAsserter { await expect(this.sb.field(fieldName)).toBeVisible(); } + async assertFieldDisabled(fieldName: string) { + await expect(this.sb.fieldArrow(fieldName, false)).toBeVisible(); + } + + async assertFieldsDisabled(fieldNames: string[]) { + for (let i = 0; i < fieldNames.length; i++) { + await this.assertFieldDisabled(fieldNames[i]); + } + } + + async assertFieldEnabled(fieldName: string) { + await expect(this.sb.fieldArrow(fieldName, true)).toBeVisible(); + } + + async assertFieldsEnabled(fieldNames: string[]) { + for (let i = 0; i < fieldNames.length; i++) { + await this.assertFieldEnabled(fieldNames[i]); + } + } + async assertFieldsInSidebar(fieldNames: string[]) { for (let i = 0; i < fieldNames.length; i++) { - await this.sb.asserter.assertFieldInSidebar(fieldNames[i]); + await this.assertFieldInSidebar(fieldNames[i]); } } diff --git a/e2e-pw/src/oss/specs/sidebar/lightning.spec.ts b/e2e-pw/src/oss/specs/sidebar/lightning.spec.ts new file mode 100644 index 0000000000..fe87631dba --- /dev/null +++ b/e2e-pw/src/oss/specs/sidebar/lightning.spec.ts @@ -0,0 +1,73 @@ +import { test as base } from "src/oss/fixtures"; +import { GridPom } from "src/oss/poms/grid"; +import { SidebarPom } from "src/oss/poms/sidebar"; +import { getUniqueDatasetNameWithPrefix } from "src/oss/utils"; + +const datasetName = getUniqueDatasetNameWithPrefix("lightning"); + +const test = base.extend<{ sidebar: SidebarPom; grid: GridPom }>({ + sidebar: async ({ page }, use) => { + await use(new SidebarPom(page)); + }, + grid: async ({ page, eventUtils }, use) => { + await use(new GridPom(page, eventUtils)); + }, +}); + +test.describe("lightning", () => { + test.beforeAll(async ({ fiftyoneLoader }) => { + await fiftyoneLoader.executePythonCode( + ` + import fiftyone as fo + + dataset = fo.Dataset("${datasetName}") + dataset.persistent = True + dataset.add_sample_field( + "ground_truth", fo.EmbeddedDocumentField, embedded_doc_type=fo.Detections + ) + dataset.add_samples( + [fo.Sample(filepath="one.png"), fo.Sample(filepath="two.png")] + ) + dataset.create_index("$**") + ` + ); + }); + + test.beforeEach(async ({ grid, page, fiftyoneLoader }) => { + await fiftyoneLoader.waitUntilGridVisible(page, datasetName); + await grid.actionsRow.toggleDisplayOptions(); + await grid.actionsRow.displayActions.setLightningMode("enable"); + }); + + test("assert enabled fields", async ({ sidebar }) => { + await sidebar.asserter.assertFieldsEnabled([ + "tags", + "metadata.mime_type", + "ground_truth", + "id", + "filepath", + ]); + await sidebar.asserter.assertFieldsDisabled([ + "_label_tags", + "metadata.size_bytes", + "metadata.width", + "metadata.height", + "metadata.num_channels", + ]); + }); + + test("assert id search no page error", async ({ + sidebar, + grid, + eventUtils, + }) => { + const entryExpandPromise = eventUtils.getEventReceivedPromiseForPredicate( + "animation-onRest", + () => true + ); + await sidebar.clickFieldDropdown("id"); + await entryExpandPromise; + await sidebar.applySearch("id", "not an id"); + await grid.assert.isLookerCountEqualTo(2); + }); +});