Skip to content

Commit

Permalink
Fix sidebar groups configuration (#4064)
Browse files Browse the repository at this point in the history 
* sidebar fix

* filling out test

* fix index

* add edge case

* rm log
  • Loading branch information
@benjaminpkane
benjaminpkane authored Feb 6, 2024
1 parent 32780da commit 757fa81
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 31 deletions.
2 changes: 1 addition & 1 deletion app/packages/core/src/components/Sidebar/Sidebar.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
120 changes: 115 additions & 5 deletions app/packages/state/src/recoil/sidebar.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ const mockFields = {
],
};

describe("ResolveGroups works", () => {
it("dataset groups should resolve when curent is undefined", () => {
const test = sidebar.resolveGroups(mockFields.sampleFields, []);
describe("test sidebar groups resolution", () => {
it("resolves with sample fields", () => {
const test = sidebar.resolveGroups(mockFields.sampleFields, [], [], []);

expect(test.length).toBe(5);
expect(test[0].name).toBe("tags");
Expand All @@ -202,7 +202,7 @@ describe("ResolveGroups works", () => {
expect(test[4].paths.length).toBe(2);
});

it("when dataset appconfig does not have sidebarGroups settings, use default settings", () => {
it("resolves merges of current client setting", () => {
const mockSidebarGroups = [
{ name: "tags", paths: [], expanded: true },
{
Expand Down Expand Up @@ -234,7 +234,8 @@ describe("ResolveGroups works", () => {
const test = sidebar.resolveGroups(
mockFields.sampleFields,
[],
mockSidebarGroups
mockSidebarGroups,
[]
);

expect(test.length).toBe(7);
Expand All @@ -245,4 +246,113 @@ describe("ResolveGroups works", () => {
expect(test[6].name).toBe("test group b");
expect(test[6].expanded).toBeFalsy();
});

it("resolves merges with dataset app config", () => {
const mockSidebarGroups = [
{
name: "labels",
paths: ["predictions", "ground_truth"],
expanded: true,
},
{
name: "primitives",
paths: ["id", "filepath", "uniqueness"],
expanded: true,
},
{ name: "other", paths: ["dict_field", "list_field"] },
{ name: "test group a", paths: [] },
{ name: "test group b", paths: [] },
{
name: "metadata",
paths: [],
expanded: true,
},
{ name: "tags", paths: [], expanded: true },
];

const test = sidebar.resolveGroups(
mockFields.sampleFields,
[],
[],
mockSidebarGroups
);

expect(test.length).toBe(7);

const tags = test[0];
expect(tags.name).toBe("tags");
expect(tags.paths).toEqual([]);

const labels = test[1];
expect(labels.name).toBe("labels");
expect(labels.paths).toEqual(["predictions", "ground_truth"]);

const primitives = test[2];
expect(primitives.name).toEqual("primitives");
expect(primitives.paths).toEqual(["id", "filepath", "uniqueness"]);
expect(primitives.expanded).toBe(true);

const other = test[3];
expect(other.name).toEqual("other");
expect(other.paths).toEqual(["dict_field", "list_field"]);

const testA = test[4];
expect(testA.name).toBe("test group a");

const testB = test[5];
expect(testB.name).toBe("test group b");

const metadata = test[6];
expect(metadata.name).toBe("metadata");
expect(metadata.expanded).toBe(true);
expect(metadata.paths).toEqual([
"metadata.size_types",
"metadata.mime_type",
"metadata.width",
"metadata.height",
"metadata.num_channels",
]);
});

it("resolves merges with improper dataset app config", () => {
const mockSidebarGroups = [
{
name: "improper",
paths: ["ground_truth"],
expanded: true,
},
];

const test = sidebar.resolveGroups(
mockFields.sampleFields,
[],
[],
mockSidebarGroups
);

const tags = test[0];
expect(tags.name).toBe("tags");
expect(tags.paths).toEqual([]);

const metadata = test[1];
expect(metadata.name).toBe("metadata");
expect(metadata.expanded).toBe(true);
expect(metadata.paths).toEqual([
"metadata.size_types",
"metadata.mime_type",
"metadata.width",
"metadata.height",
"metadata.num_channels",
]);

const improper = test[2];
expect(improper.name).toBe("improper");
expect(improper.paths).toEqual(["ground_truth"]);
expect(improper.expanded).toBe(true);

const labels = test[3];
expect(labels.name).toBe("labels");
expect(labels.paths).toEqual(["predictions"]);
expect(labels.expanded).toBe(true);
});
});
69 changes: 47 additions & 22 deletions app/packages/state/src/recoil/sidebar.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
setSidebarGroups,
setSidebarGroupsMutation,
sidebarGroupsFragment,
sidebarGroupsFragment$data,
sidebarGroupsFragment$key,
} from "@fiftyone/relay";
import {
Expand Down Expand Up @@ -164,14 +163,14 @@ export const RESERVED_GROUPS = new Set([

const LABELS = withPath(LABELS_PATH, VALID_LABEL_TYPES);

const DEFAULT_IMAGE_GROUPS = [
const DEFAULT_IMAGE_GROUPS: State.SidebarGroup[] = [
{ name: "tags", paths: [] },
{ name: "metadata", paths: [] },
{ name: "labels", paths: [] },
{ name: "primitives", paths: [] },
];

const DEFAULT_VIDEO_GROUPS = [
const DEFAULT_VIDEO_GROUPS: State.SidebarGroup[] = [
{ name: "tags", paths: [] },
{ name: "metadata", paths: [] },
{ name: "labels", paths: [] },
Expand All @@ -184,17 +183,18 @@ const NONE = [null, undefined];
export const resolveGroups = (
sampleFields: StrictField[],
frameFields: StrictField[],
sidebarGroups?: State.SidebarGroup[],
config: NonNullable<
sidebarGroupsFragment$data["appConfig"]
>["sidebarGroups"] = []
currentGroups: State.SidebarGroup[],
configGroups: State.SidebarGroup[]
): State.SidebarGroup[] => {
let groups = sidebarGroups?.length
? sidebarGroups
let groups = currentGroups.length
? currentGroups
: configGroups.length
? configGroups
: frameFields.length
? DEFAULT_VIDEO_GROUPS
: DEFAULT_IMAGE_GROUPS;
const expanded = config.reduce((map, { name, expanded }) => {

const expanded = configGroups.reduce((map, { name, expanded }) => {
map[name] = expanded;
return map;
}, {});
Expand All @@ -205,24 +205,47 @@ export const resolveGroups = (
: { ...group };
});

const metadata = groups.find(({ name }) => name === "metadata");
if (!metadata) {
groups.unshift({
name: "metadata",
expanded: true,
paths: [],
});
}

const tags = groups.find(({ name }) => name === "tags");
groups = groups.filter(({ name }) => name !== "tags");
groups.unshift({
name: "tags",
expanded: tags?.expanded,
paths: [],
});

const present = new Set<string>(groups.map(({ paths }) => paths).flat());
const updater = groupUpdater(
groups,
buildSchema(sampleFields, frameFields),
present
);

updater("labels", fieldsMatcher(sampleFields, labelsMatcher(), present));
updater(
"labels",
fieldsMatcher(sampleFields, labelsMatcher(), present),
true
);

frameFields.length &&
updater(
"frame labels",
fieldsMatcher(frameFields, labelsMatcher(), present, "frames.")
fieldsMatcher(frameFields, labelsMatcher(), present, "frames."),
true
);

updater(
"primitives",
fieldsMatcher(sampleFields, primitivesMatcher, present)
fieldsMatcher(sampleFields, primitivesMatcher, present),
true
);

sampleFields.filter(groupFilter).forEach(({ fields, name }) => {
Expand All @@ -237,7 +260,8 @@ export const resolveGroups = (
present.add(`frames.${name}`);
updater(
`frames.${name}`,
fieldsMatcher(fields || [], () => true, present, `frames.${name}.`)
fieldsMatcher(fields || [], () => true, present, `frames.${name}.`),
true
);
});

Expand All @@ -262,13 +286,13 @@ const groupUpdater = (
groups[i].paths = filterPaths(groups[i].paths, schema);
}

return (name: string, paths: string[]) => {
return (name: string, paths: string[], expanded = false) => {
if (paths.length === 0) return;
paths.forEach((path) => present.add(path));

const index = groupNames.indexOf(name);
if (index < 0) {
groups.push({ name, paths, expanded: false });
groups.push({ name, paths, expanded });
groupNames.push(name);
return;
}
Expand All @@ -279,13 +303,11 @@ const groupUpdater = (
};

export const [resolveSidebarGroups, sidebarGroupsDefinition] = (() => {
let config: NonNullable<
sidebarGroupsFragment$data["appConfig"]
>["sidebarGroups"] = [];
let configGroups: State.SidebarGroup[] = [];
let current: State.SidebarGroup[] = [];
return [
(sampleFields: StrictField[], frameFields: StrictField[]) => {
return resolveGroups(sampleFields, frameFields, current, config);
return resolveGroups(sampleFields, frameFields, current, configGroups);
},
graphQLSyncFragmentAtomFamily<
sidebarGroupsFragment$key,
Expand All @@ -297,7 +319,10 @@ export const [resolveSidebarGroups, sidebarGroupsDefinition] = (() => {
keys: ["dataset"],
sync: (modal) => !modal,
read: (data, prev) => {
config = data.appConfig?.sidebarGroups || [];
configGroups = (data.appConfig?.sidebarGroups || []).map((group) => ({
...group,
paths: [...group.paths],
}));
current = resolveGroups(
collapseFields(
readFragment(
Expand All @@ -310,7 +335,7 @@ export const [resolveSidebarGroups, sidebarGroupsDefinition] = (() => {
.frameFields
),
data.name === prev?.name ? current : [],
config
configGroups
);

return current;
Expand Down
4 changes: 1 addition & 3 deletions app/packages/utilities/src/styles.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ scrollbar-color: ${({ theme }) => theme.text.tertiary} ${({ theme }) =>
border: solid 4px transparent ${theme.text.tertiary};
}
@-moz-document url-prefix() {
padding-right: 16px;
}
::-webkit-scrollbar-thumb {
box-shadow: inset 0 0 10px 10px transparent;
Expand Down

0 comments on commit 757fa81

Please sign in to comment.