Skip to content

Commit

Permalink
Sorting works
Browse files Browse the repository at this point in the history
  • Loading branch information
rafpaf committed May 1, 2024
1 parent a755312 commit e05d930
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 46 deletions.
3 changes: 2 additions & 1 deletion frontend/src/metabase-types/api/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ export type SearchResponse<
export type CollectionEssentials = Pick<
Collection,
"id" | "name" | "authority_level" | "type"
>;
> &
Partial<Collection>;

export type SearchResultId =
| CollectionId
Expand Down
41 changes: 34 additions & 7 deletions frontend/src/metabase/browse/components/BrowseModels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import { color } from "metabase/lib/colors";
import { useDispatch } from "metabase/lib/redux";
import { PLUGIN_CONTENT_VERIFICATION } from "metabase/plugins";
import { Box, Flex, Group, Icon, Stack, Title } from "metabase/ui";
import type { SearchRequest } from "metabase-types/api";
import type {
CollectionEssentials,
type SearchRequest,

Check failure on line 16 in frontend/src/metabase/browse/components/BrowseModels.tsx

View workflow job for this annotation

GitHub Actions / fe-type-check

The 'type' modifier cannot be used on a named import when 'import type' is used on its import statement.
} from "metabase-types/api";
import { SortDirection } from "metabase-types/api";

import { filterModels, type ActualModelFilters } from "../utils";

Expand All @@ -24,6 +28,7 @@ import {
} from "./BrowseApp.styled";
import { ModelExplanationBanner } from "./ModelExplanationBanner";
import { ModelsTable } from "./ModelsTable";
import { getCollectionPathString } from "./utils";

const { availableModelFilters, useModelFilterSettings } =
PLUGIN_CONTENT_VERIFICATION;
Expand Down Expand Up @@ -73,20 +78,19 @@ export const BrowseModelsBody = ({
const dispatch = useDispatch();
const [sortingOptions, setSortingOptions] = useState<SortingOptions>({
sort_column: "name",
sort_direction: "asc",
sort_direction: SortDirection.Asc,
});

const query: SearchRequest = {
models: ["dataset"], // 'model' in the sense of 'type of thing'
model_ancestors: true,
filter_items_in_personal_collection: "exclude",
...sortingOptions,
};

const { data, error, isLoading } = useSearchQuery(query);
const unfilteredModels = data?.data;
const unfilteredModels = useMemo(() => data?.data, [data]);

const models = useMemo(
const filteredModels = useMemo(
() =>
filterModels(
unfilteredModels || [],
Expand All @@ -96,7 +100,30 @@ export const BrowseModelsBody = ({
[unfilteredModels, actualModelFilters],
);

const wrappedModels = models.map(model => Search.wrapEntity(model, dispatch));
const sortedModels = useMemo(() => {
const { sort_column, sort_direction } = sortingOptions;
const sorted = _.sortBy(filteredModels, model => {
if (sort_column === "collection") {
const collection: CollectionEssentials = model.collection;
return getCollectionPathString(collection);
}
if (sort_column in model) {
return model[sort_column as keyof typeof model];
} else {
console.error("Invalid sort column", sort_column);
return null;
}
});
if (sort_direction === SortDirection.Desc) {
sorted.reverse();
}
return sorted;
}, [filteredModels, sortingOptions]);

const wrappedModels = useMemo(
() => sortedModels.map(model => Search.wrapEntity(model, dispatch)),
[sortedModels, dispatch],
);

if (error || isLoading) {
return (
Expand All @@ -108,7 +135,7 @@ export const BrowseModelsBody = ({
);
}

if (models.length) {
if (filteredModels.length) {
return (
<Stack spacing="md" mb="lg">
<ModelExplanationBanner />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import classNames from "classnames";
import type { FC } from "react";
import { forwardRef, useLayoutEffect, useRef, useState } from "react";
import { c } from "ttag";

import { ResponsiveContainer } from "metabase/components/ResponsiveContainer/ResponsiveContainer";
import { useAreAnyTruncated } from "metabase/hooks/use-is-truncated";
Expand All @@ -17,12 +16,9 @@ import {
Breadcrumb,
CollectionBreadcrumbsWrapper,
} from "./CollectionBreadcrumbsWithTooltip.styled";
import { pathSeparatorChar } from "./constants";
import type { RefProp } from "./types";
import { getBreadcrumbMaxWidths } from "./utils";

const separatorCharacter = c(
"Character that separates the names of collections in a path, as in 'Europe / Belgium / Antwerp' or 'Products / Prototypes / Alice's Prototypes'",
).t`/`;
import { getBreadcrumbMaxWidths, getCollectionPathString } from "./utils";

export const CollectionBreadcrumbsWithTooltip = ({
collection,
Expand All @@ -32,9 +28,7 @@ export const CollectionBreadcrumbsWithTooltip = ({
containerName: string;
}) => {
const collections = (collection.effective_ancestors || []).concat(collection);
const pathString = collections
.map(coll => getCollectionName(coll))
.join(` ${separatorCharacter} `);
const pathString = getCollectionPathString(collection);
const ellipsifyPath = collections.length > 2;
const shownCollections = ellipsifyPath
? [collections[0], collections[collections.length - 1]]
Expand Down Expand Up @@ -147,6 +141,6 @@ Ellipsis.displayName = "Ellipsis";

const PathSeparator = () => (
<Text color="text-light" mx="xs" py={1}>
{separatorCharacter}
{pathSeparatorChar}
</Text>
);
10 changes: 2 additions & 8 deletions frontend/src/metabase/browse/components/ModelsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,9 @@ export const ModelsTable = ({
sortingOptions={sortingOptions}
onSortingOptionsChange={onSortingOptionsChange}
/>
<ColumnHeader {...descriptionProps}>{t`Description`}</ColumnHeader>
<SortableColumnHeader
name="description"
sortingOptions={sortingOptions}
onSortingOptionsChange={onSortingOptionsChange}
{...descriptionProps}
>
{t`Description`}
</SortableColumnHeader>
<SortableColumnHeader
name="collection"
sortingOptions={sortingOptions}
onSortingOptionsChange={onSortingOptionsChange}
{...collectionProps}
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/metabase/browse/components/constants.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { c } from "ttag";

export const pathSeparatorChar = c(
"Character that separates the names of collections in a path, as in 'Europe / Belgium / Antwerp' or 'Products / Prototypes / Alice's Prototypes'",
).t`/`;
16 changes: 15 additions & 1 deletion frontend/src/metabase/browse/components/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { t } from "ttag";

import { isRootCollection } from "metabase/collections/utils";
import type { CollectionItem, Collection } from "metabase-types/api";
import type {
CollectionItem,
Collection,
CollectionEssentials,
} from "metabase-types/api";

import { getCollectionName } from "../utils";

import { pathSeparatorChar } from "./constants";

export const getBreadcrumbMaxWidths = (
collections: Collection["effective_ancestors"],
totalUnitsOfWidthAvailable: number,
Expand Down Expand Up @@ -43,3 +49,11 @@ export const getModelDescription = (item: CollectionItem) => {
return item.description;
}
};

export const getCollectionPathString = (collection: CollectionEssentials) => {
const collections = (collection.effective_ancestors || []).concat(collection);
const pathString = collections
.map(coll => getCollectionName(coll))
.join(` ${pathSeparatorChar} `);
return pathString;
};
41 changes: 24 additions & 17 deletions frontend/src/metabase/components/ItemsTable/BaseItemsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
useCallback,
type HTMLAttributes,
type PropsWithChildren,
useMemo,
} from "react";

import type { ActionMenuProps } from "metabase/collections/components/ActionMenu/ActionMenu";
Expand Down Expand Up @@ -35,7 +36,7 @@ import type { ResponsiveProps } from "./utils";

export type SortingOptions = {
sort_column: string;
sort_direction: "asc" | "desc";
sort_direction: SortDirection;
};

export type SortableColumnHeaderProps = {
Expand All @@ -45,31 +46,37 @@ export type SortableColumnHeaderProps = {
} & PropsWithChildren<Partial<HTMLAttributes<HTMLDivElement>>>;

export const SortableColumnHeader = ({
name = "",
sortingOptions = {
sort_column: "",
sort_direction: SortDirection.Asc,
},
name,
sortingOptions,
onSortingOptionsChange,
children,
hideAtContainerBreakpoint,
containerName,
...props
}: SortableColumnHeaderProps & ResponsiveProps) => {
const isSortable = !!onSortingOptionsChange;
const isSortingThisColumn = sortingOptions.sort_column === name;
const isSortable = !!onSortingOptionsChange && !!name;
const isSortingThisColumn = sortingOptions?.sort_column === name;
const direction = isSortingThisColumn
? sortingOptions.sort_direction
? sortingOptions?.sort_direction
: SortDirection.Desc;

const onSortingControlClick = () => {
const nextDirection =
direction === SortDirection.Asc ? SortDirection.Desc : SortDirection.Asc;
onSortingOptionsChange?.({
sort_column: name,
sort_direction: nextDirection,
});
};
const onSortingControlClick = useMemo(() => {
if (!isSortable) {
return undefined;
}
const handler = () => {
const nextDirection =
direction === SortDirection.Asc
? SortDirection.Desc
: SortDirection.Asc;
const newSortingOptions = {
sort_column: name,
sort_direction: nextDirection,
};
onSortingOptionsChange?.(newSortingOptions);
};
return handler;
}, [direction, isSortable, name, onSortingOptionsChange]);

return (
<ColumnHeader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
DEFAULT_TIME_STYLE,
} from "metabase/lib/formatting/datetime-utils";
import type { IconName } from "metabase/ui";
import type { CollectionItem } from "metabase-types/api";
import { SortDirection, type CollectionItem } from "metabase-types/api";
import { createMockCollection } from "metabase-types/api/mocks";

import type { BaseItemsTableProps } from "./BaseItemsTable";
Expand Down Expand Up @@ -64,7 +64,10 @@ describe("BaseItemsTable", () => {
component={() => (
<BaseItemsTable
items={items}
sortingOptions={{ sort_column: "name", sort_direction: "asc" }}
sortingOptions={{
sort_column: "name",
sort_direction: SortDirection.Asc,
}}
onSortingOptionsChange={jest.fn()}
{...props}
/>
Expand Down

0 comments on commit e05d930

Please sign in to comment.