From 81037fa6ff282fc671a4c89ab8811cdc6479da09 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Fri, 27 Sep 2024 13:36:32 -0400 Subject: [PATCH] Render summary fields in modal sidebar (#4851) * render summary fields in modal sidebar * lint log * fix urls * e2e summary fields * e2e fixes --- .../Sidebar/Entries/PathValueEntry.tsx | 139 +++++++++++++++--- app/packages/state/src/recoil/schema.ts | 10 +- app/packages/utilities/src/index.ts | 24 +-- e2e-pw/src/oss/poms/modal/modal-sidebar.ts | 12 ++ .../specs/smoke-tests/summary-fields.spec.ts | 70 +++++++++ 5 files changed, 226 insertions(+), 29 deletions(-) create mode 100644 e2e-pw/src/oss/specs/smoke-tests/summary-fields.spec.ts diff --git a/app/packages/core/src/components/Sidebar/Entries/PathValueEntry.tsx b/app/packages/core/src/components/Sidebar/Entries/PathValueEntry.tsx index 4c62764487..d4114821fa 100644 --- a/app/packages/core/src/components/Sidebar/Entries/PathValueEntry.tsx +++ b/app/packages/core/src/components/Sidebar/Entries/PathValueEntry.tsx @@ -1,6 +1,11 @@ import { LoadingDots, useTheme } from "@fiftyone/components"; import * as fos from "@fiftyone/state"; -import { formatPrimitive, makePseudoField } from "@fiftyone/utilities"; +import type { Primitive, Schema } from "@fiftyone/utilities"; +import { + EMBEDDED_DOCUMENT_FIELD, + formatPrimitive, + makePseudoField, +} from "@fiftyone/utilities"; import { KeyboardArrowDown, KeyboardArrowUp } from "@mui/icons-material"; import { useSpring } from "@react-spring/core"; import React, { Suspense, useMemo, useState } from "react"; @@ -51,6 +56,10 @@ const ScalarDiv = styled.div` &.expanded > div { white-space: unset; } + + & a { + color: ${({ theme }) => theme.text.primary}; + } `; const ScalarValueEntry = ({ @@ -116,9 +125,11 @@ const ListContainer = styled(ScalarDiv)` color: ${({ theme }) => theme.text.secondary}; margin-top: 0.25rem; padding: 0.25rem 0.5rem; + display: flex; + flex-direction: column; + row-gap: 0.5rem; & > div { - margin-bottom: 0.5rem; white-space: unset; } `; @@ -186,6 +197,7 @@ const ListValueEntry = ({ { event.preventDefault(); @@ -226,20 +238,31 @@ const LengthLoadable = ({ path }: { path: string }) => { }; const ListLoadable = ({ path }: { path: string }) => { - const data = useData(path); + const data = useData(path); + const { fields, ftype, subfield } = fos.useAssertedRecoilValue( + fos.field(path) + ); + const timeZone = useRecoilValue(fos.timeZone); + + const field = subfield || ftype; + if (!field) { + throw new Error(`expected an ftype for ${path}`); + } + const values = useMemo(() => { - return data - ? Array.from(data).map((value) => prettify(value as string)) - : []; - }, [data]); + return Array.from(data || []).map((value) => + format({ fields, ftype: field, value, timeZone }) + ); + }, [data, field, fields, timeZone]); + return ( - + {values.map((v, i) => ( -
- {v} +
+ {v === null ? "None" : v}
))} - {values.length === 0 && <>No results} + {values.length === 0 && "No results"} ); }; @@ -263,9 +286,9 @@ const SlicesListLoadable = ({ path }: { path: string }) => { {slice}
{(data || []).map((value, i) => ( -
{prettify(value as string)}
+
{prettify(value as string)}
))} - {(!data || !data.length) && <>No results} + {(!data || !data.length) && "No results"} ); })} @@ -286,7 +309,7 @@ const SlicesLoadable = ({ path }: { path: string }) => { <> {Object.entries(values).map(([slice, value], i) => { const none = value === null || value === undefined; - const formatted = formatPrimitive({ ftype, value, timeZone }); + const formatted = format({ ftype, value, timeZone }); const add = none ? { color } : {}; return ( @@ -297,7 +320,7 @@ const SlicesLoadable = ({ path }: { path: string }) => { columnGap: "0.5rem", marginBottom: "0.5rem", }} - key={i} + key={i.toString()} >
{slice}
(path: string) => { const Loadable = ({ path }: { path: string }) => { const value = useData(path); const none = value === null || value === undefined; - const { ftype } = useRecoilValue(fos.field(path)) ?? makePseudoField(path); + const { fields, ftype } = + useRecoilValue(fos.field(path)) ?? makePseudoField(path); const color = useRecoilValue(fos.pathColor(path)); const timeZone = useRecoilValue(fos.timeZone); - const formatted = formatPrimitive({ ftype, value, timeZone }); + + const formatted = useMemo( + () => format({ fields, ftype, timeZone, value }), + [fields, ftype, timeZone, value] + ); return (
e.stopPropagation()} onClick={(e) => e.stopPropagation()} style={none ? { color } : {}} title={typeof formatted === "string" ? formatted : undefined} @@ -439,4 +468,80 @@ const PathValueEntry = ({ ); }; +interface PrimitivesObject { + [key: string]: Primitive; +} + +type Primitives = Primitive | PrimitivesObject; + +const format = ({ + fields, + ftype, + timeZone, + value, +}: { + fields?: Schema; + ftype: string; + timeZone: string; + value: Primitives; +}) => { + if (ftype === EMBEDDED_DOCUMENT_FIELD && typeof value === "object") { + return formatObject({ fields, timeZone, value: value as object }); + } + + return formatPrimitiveOrURL({ ftype, value: value as Primitive, timeZone }); +}; + +const formatPrimitiveOrURL = (params: { + fields?: Schema; + ftype: string; + timeZone: string; + value: Primitive; +}) => { + const result = formatPrimitive(params); + + return result instanceof URL ? ( + + {result.toString()} + + ) : ( + result + ); +}; + +const formatObject = ({ + fields, + timeZone, + value, +}: { + fields?: Schema; + timeZone: string; + value: object; +}) => { + return Object.entries(value) + .map(([k, v]) => { + if (!fields?.[k]?.ftype) { + return null; + } + + const text = formatPrimitiveOrURL({ + ftype: fields?.[k]?.ftype, + timeZone, + value: v, + }); + + return ( +
+ {k} + {text} +
+ ); + }) + .filter((entry) => Boolean(entry)); +}; + export default React.memo(PathValueEntry); diff --git a/app/packages/state/src/recoil/schema.ts b/app/packages/state/src/recoil/schema.ts index 715562ba86..2a746f44ac 100644 --- a/app/packages/state/src/recoil/schema.ts +++ b/app/packages/state/src/recoil/schema.ts @@ -17,13 +17,13 @@ import { LABEL_LISTS, LABEL_LISTS_MAP, LIST_FIELD, - meetsFieldType, OBJECT_ID_FIELD, + STRING_FIELD, Schema, StrictField, - STRING_FIELD, VALID_NUMERIC_TYPES, VALID_PRIMITIVE_TYPES, + meetsFieldType, withPath, } from "@fiftyone/utilities"; import { RecoilState, selector, selectorFamily } from "recoil"; @@ -786,7 +786,11 @@ export const isOfDocumentFieldList = selectorFamily({ get: (path: string) => ({ get }) => { - const f = get(field(path.split(".")[0])); + const parent = path.split(".").slice(0, -1).join("."); + if (!parent) { + return false; + } + const f = get(field(parent)); return [ DYNAMIC_EMBEDDED_DOCUMENT_FIELD, diff --git a/app/packages/utilities/src/index.ts b/app/packages/utilities/src/index.ts index 8a85bdd8fa..3f1618a21c 100644 --- a/app/packages/utilities/src/index.ts +++ b/app/packages/utilities/src/index.ts @@ -3,13 +3,13 @@ import _ from "lodash"; import mime from "mime"; import { Field } from "./schema"; +export * from "./Resource"; export * from "./buffer-manager"; export * from "./color"; export * from "./errors"; export * from "./fetch"; export * from "./order"; export * from "./paths"; -export * from "./Resource"; export * from "./schema"; export * as styles from "./styles"; export * from "./type-check"; @@ -618,6 +618,13 @@ export const formatDate = (timeStamp: number): string => { .replaceAll("/", "-"); }; +export type Primitive = + | number + | null + | string + | undefined + | { datetime: number }; + export const formatPrimitive = ({ ftype, timeZone, @@ -625,24 +632,23 @@ export const formatPrimitive = ({ }: { ftype: string; timeZone: string; - value: unknown; + value: Primitive; }) => { - if (value === null || value === undefined) return undefined; + if (value === null || value === undefined) return null; switch (ftype) { case FRAME_SUPPORT_FIELD: - value = `[${value[0]}, ${value[1]}]`; - break; + return `[${value[0]}, ${value[1]}]`; case DATE_FIELD: // @ts-ignore - value = formatDate(value.datetime as number); - break; + return formatDate(value?.datetime as number); case DATE_TIME_FIELD: // @ts-ignore - value = formatDateTime(value.datetime as number, timeZone); + return formatDateTime(value?.datetime as number, timeZone); } - return prettify(String(value)); + // @ts-ignore + return prettify(value); }; export const makePseudoField = (path: string): Field => ({ diff --git a/e2e-pw/src/oss/poms/modal/modal-sidebar.ts b/e2e-pw/src/oss/poms/modal/modal-sidebar.ts index 665c0eb1ee..7a7ae699cc 100644 --- a/e2e-pw/src/oss/poms/modal/modal-sidebar.ts +++ b/e2e-pw/src/oss/poms/modal/modal-sidebar.ts @@ -126,6 +126,18 @@ class SidebarAsserter { ); } + async verifyObject(key: string, obj: { [key: string]: string }) { + const locator = this.modalSidebarPom.getSidebarEntry(key); + + for (const k in obj) { + const v = obj[k]; + const entry = locator.getByTestId(`key-value-${k}-${v}`); + + await expect(entry.getByTestId(`key-${k}`)).toHaveText(k); + await expect(entry.getByTestId(`value-${v}`)).toHaveText(v); + } + } + async verifyLabelTagCount(count: number) { await this.modalSidebarPom.page.waitForFunction( (count_) => { diff --git a/e2e-pw/src/oss/specs/smoke-tests/summary-fields.spec.ts b/e2e-pw/src/oss/specs/smoke-tests/summary-fields.spec.ts new file mode 100644 index 0000000000..42d492153d --- /dev/null +++ b/e2e-pw/src/oss/specs/smoke-tests/summary-fields.spec.ts @@ -0,0 +1,70 @@ +import { test as base } from "src/oss/fixtures"; +import { GridPom } from "src/oss/poms/grid"; +import { ModalPom } from "src/oss/poms/modal"; +import { getUniqueDatasetNameWithPrefix } from "src/oss/utils"; + +const test = base.extend<{ grid: GridPom; modal: ModalPom }>({ + grid: async ({ page, eventUtils }, use) => { + await use(new GridPom(page, eventUtils)); + }, + modal: async ({ page, eventUtils }, use) => { + await use(new ModalPom(page, eventUtils)); + }, +}); + +const datasetName = getUniqueDatasetNameWithPrefix("summary-fields"); + +test.describe("summary fields", () => { + test.beforeAll(async ({ fiftyoneLoader }) => { + await fiftyoneLoader.executePythonCode(` + import fiftyone as fo + + dataset = fo.Dataset("${datasetName}") + dataset.persistent = True + dataset.add_sample( + fo.Sample( + filepath=f"image.png", + summary=fo.DynamicEmbeddedDocument(one="two", three="four"), + summaries=[ + fo.DynamicEmbeddedDocument(five="six", seven="eight"), + fo.DynamicEmbeddedDocument(nine="ten"), + ], + ) + ) + dataset.app_config.sidebar_groups = [ + fo.SidebarGroupDocument( + name="summaries", paths=["summary", "summaries"], expanded=True + ) + ] + dataset.save() + dataset.add_dynamic_sample_fields() + `); + }); + + test("modal sidebar summary fields render", async ({ + eventUtils, + fiftyoneLoader, + grid, + modal, + page, + }) => { + await fiftyoneLoader.waitUntilGridVisible(page, datasetName); + await grid.openFirstSample(); + await modal.waitForSampleLoadDomAttribute(true); + await modal.sidebar.assert.verifyObject("summary", { + one: "two", + three: "four", + }); + const entryExpandPromise = eventUtils.getEventReceivedPromiseForPredicate( + "animation-onRest", + () => true + ); + await modal.sidebar.clickFieldDropdown("summaries"); + await entryExpandPromise; + await modal.sidebar.assert.verifyObject("summaries", { + five: "six", + seven: "eight", + nine: "ten", + }); + }); +});