From 7ce866130efaa1a331932d3c5cbbf4f3559898b3 Mon Sep 17 00:00:00 2001 From: Ryan Laurie <30528226+iethree@users.noreply.github.com> Date: Thu, 9 May 2024 05:49:16 -0600 Subject: [PATCH] Remove Dead Pickers (#41801) * update e2e tests * rework linked entity picker * remove deprecated pickers * remove dead picker code * another one bites the dust * remove create collection on the go * update linkedEntityPicker unit tests- * update timeline move modal * remove unused import --- .../timelines-collection.cy.spec.js | 8 +- e2e/test/scenarios/question/new.cy.spec.js | 4 +- .../CreateCollectionForm.tsx | 2 +- .../FormCollectionPicker.styled.tsx | 11 - .../FormCollectionPicker.tsx | 2 +- .../common/components/EntityPicker/types.ts | 2 + .../components/QuestionPickerModal.tsx | 7 + .../AddToDashSelectDashModal.styled.tsx | 10 - .../metabase/containers/CollectionPicker.jsx | 32 -- .../metabase/containers/CollectionSelect.jsx | 24 -- .../CreateCollectionOnTheGo.styled.tsx | 7 - .../containers/CreateCollectionOnTheGo.tsx | 91 ----- .../metabase/containers/DashboardPicker.tsx | 32 -- .../containers/ItemPicker/Item.styled.tsx | 82 ---- .../metabase/containers/ItemPicker/Item.tsx | 82 ---- .../ItemPicker/ItemPicker.styled.tsx | 50 --- .../containers/ItemPicker/ItemPicker.tsx | 293 -------------- .../ItemPicker/ItemPicker.unit.spec.js | 383 ------------------ .../containers/ItemPicker/ItemPickerView.tsx | 226 ----------- .../metabase/containers/ItemPicker/index.ts | 3 - .../containers/ItemPicker/test-utils.js | 27 -- .../metabase/containers/ItemPicker/types.ts | 29 -- .../metabase/containers/QuestionPicker.jsx | 26 -- .../metabase/containers/QuestionSelect.tsx | 14 - .../SaveQuestionModal/SaveQuestionModal.tsx | 144 ++++--- .../LinkedEntityPicker/LinkedEntityPicker.tsx | 79 ++-- .../LinkedEntityPicker.unit.spec.tsx | 102 +++-- .../containers/CopyDashboardForm.tsx | 2 +- .../containers/CreateDashboardForm.tsx | 2 +- .../containers/CreateDashboardModal.tsx | 28 +- .../entities/containers/EntityCopyModal.tsx | 27 +- .../FormModelPicker.styled.tsx | 11 - .../metabase/timelines/collections/routes.tsx | 1 + .../MoveTimelineModal.styled.tsx | 17 - .../MoveTimelineModal/MoveTimelineModal.tsx | 64 ++- 35 files changed, 252 insertions(+), 1672 deletions(-) delete mode 100644 frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.styled.tsx delete mode 100644 frontend/src/metabase/containers/AddToDashSelectDashModal/AddToDashSelectDashModal.styled.tsx delete mode 100644 frontend/src/metabase/containers/CollectionPicker.jsx delete mode 100644 frontend/src/metabase/containers/CollectionSelect.jsx delete mode 100644 frontend/src/metabase/containers/CreateCollectionOnTheGo.styled.tsx delete mode 100644 frontend/src/metabase/containers/CreateCollectionOnTheGo.tsx delete mode 100644 frontend/src/metabase/containers/DashboardPicker.tsx delete mode 100644 frontend/src/metabase/containers/ItemPicker/Item.styled.tsx delete mode 100644 frontend/src/metabase/containers/ItemPicker/Item.tsx delete mode 100644 frontend/src/metabase/containers/ItemPicker/ItemPicker.styled.tsx delete mode 100644 frontend/src/metabase/containers/ItemPicker/ItemPicker.tsx delete mode 100644 frontend/src/metabase/containers/ItemPicker/ItemPicker.unit.spec.js delete mode 100644 frontend/src/metabase/containers/ItemPicker/ItemPickerView.tsx delete mode 100644 frontend/src/metabase/containers/ItemPicker/index.ts delete mode 100644 frontend/src/metabase/containers/ItemPicker/test-utils.js delete mode 100644 frontend/src/metabase/containers/ItemPicker/types.ts delete mode 100644 frontend/src/metabase/containers/QuestionPicker.jsx delete mode 100644 frontend/src/metabase/containers/QuestionSelect.tsx delete mode 100644 frontend/src/metabase/models/containers/FormModelPicker/FormModelPicker.styled.tsx delete mode 100644 frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.styled.tsx diff --git a/e2e/test/scenarios/organization/timelines-collection.cy.spec.js b/e2e/test/scenarios/organization/timelines-collection.cy.spec.js index 09fd049663f..46a35a40663 100644 --- a/e2e/test/scenarios/organization/timelines-collection.cy.spec.js +++ b/e2e/test/scenarios/organization/timelines-collection.cy.spec.js @@ -9,6 +9,7 @@ import { getFullName, popover, modal, + entityPickerModal, } from "e2e/support/helpers"; const { admin } = USERS; @@ -462,11 +463,10 @@ describe("scenarios > organization > timelines > collection", () => { cy.visit("/collection/root/timelines"); openMenu("Our analytics events"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Move timeline").click(); + popover().findByText("Move timeline").click(); - modal().within(() => { - cy.findByText("My personal collection").click(); + entityPickerModal().within(() => { + cy.findByText("Bobby Tables's Personal Collection").click(); cy.button("Move").click(); cy.wait("@updateTimeline"); }); diff --git a/e2e/test/scenarios/question/new.cy.spec.js b/e2e/test/scenarios/question/new.cy.spec.js index f4850b839d0..696bd51232c 100644 --- a/e2e/test/scenarios/question/new.cy.spec.js +++ b/e2e/test/scenarios/question/new.cy.spec.js @@ -401,7 +401,9 @@ describe("scenarios > question > new", () => { entityPickerModal().within(() => { cy.findByText("Add this question to a dashboard").should("be.visible"); - cy.findByText(myPersonalCollectionName).should("be.visible"); + cy.findByText(/bobby tables's personal collection/i).should( + "be.visible", + ); cy.findByText(/our analytics/i).should("not.exist"); }); }); diff --git a/frontend/src/metabase/collections/components/CreateCollectionForm/CreateCollectionForm.tsx b/frontend/src/metabase/collections/components/CreateCollectionForm/CreateCollectionForm.tsx index 2ee54d281a1..4f8aa6b7399 100644 --- a/frontend/src/metabase/collections/components/CreateCollectionForm/CreateCollectionForm.tsx +++ b/frontend/src/metabase/collections/components/CreateCollectionForm/CreateCollectionForm.tsx @@ -6,7 +6,7 @@ import _ from "underscore"; import * as Yup from "yup"; import FormCollectionPicker from "metabase/collections/containers/FormCollectionPicker"; -import type { FilterItemsInPersonalCollection } from "metabase/containers/ItemPicker"; +import type { FilterItemsInPersonalCollection } from "metabase/common/components/EntityPicker"; import Button from "metabase/core/components/Button"; import FormErrorMessage from "metabase/core/components/FormErrorMessage"; import FormFooter from "metabase/core/components/FormFooter"; diff --git a/frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.styled.tsx b/frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.styled.tsx deleted file mode 100644 index ad80cf0d02f..00000000000 --- a/frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.styled.tsx +++ /dev/null @@ -1,11 +0,0 @@ -import styled from "@emotion/styled"; - -import ItemPicker from "metabase/containers/ItemPicker"; - -export const MIN_POPOVER_WIDTH = 300; - -export const PopoverItemPicker = styled(ItemPicker)<{ width: number }>` - width: ${({ width = MIN_POPOVER_WIDTH }) => width}px; - padding: 1rem; - overflow: auto; -`; diff --git a/frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.tsx b/frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.tsx index ed3a0c0e945..b4d6977df4b 100644 --- a/frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.tsx +++ b/frontend/src/metabase/collections/containers/FormCollectionPicker/FormCollectionPicker.tsx @@ -12,8 +12,8 @@ import type { CollectionPickerOptions, } from "metabase/common/components/CollectionPicker"; import { CollectionPickerModal } from "metabase/common/components/CollectionPicker"; +import type { FilterItemsInPersonalCollection } from "metabase/common/components/EntityPicker"; import CollectionName from "metabase/containers/CollectionName"; -import type { FilterItemsInPersonalCollection } from "metabase/containers/ItemPicker"; import SnippetCollectionName from "metabase/containers/SnippetCollectionName"; import FormField from "metabase/core/components/FormField"; import Collections from "metabase/entities/collections"; diff --git a/frontend/src/metabase/common/components/EntityPicker/types.ts b/frontend/src/metabase/common/components/EntityPicker/types.ts index 50e797e93f6..c330cefae33 100644 --- a/frontend/src/metabase/common/components/EntityPicker/types.ts +++ b/frontend/src/metabase/common/components/EntityPicker/types.ts @@ -46,3 +46,5 @@ export type ListProps< options: Options; shouldDisableItem?: (item: Item) => boolean; }; + +export type FilterItemsInPersonalCollection = "only" | "exclude"; diff --git a/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPickerModal.tsx b/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPickerModal.tsx index c19e975a3d4..7ddffa05158 100644 --- a/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPickerModal.tsx +++ b/frontend/src/metabase/common/components/QuestionPicker/components/QuestionPickerModal.tsx @@ -117,6 +117,13 @@ export const QuestionPickerModal = ({ initialValue={value} tabs={filteredTabs} options={options} + searchParams={ + options.showRootCollection === false + ? { filter_items_in_personal_collection: "only" } + : options.showPersonalCollections === false + ? { filter_items_in_personal_collection: "exclude" } + : undefined + } searchResultFilter={results => results} actionButtons={[]} /> diff --git a/frontend/src/metabase/containers/AddToDashSelectDashModal/AddToDashSelectDashModal.styled.tsx b/frontend/src/metabase/containers/AddToDashSelectDashModal/AddToDashSelectDashModal.styled.tsx deleted file mode 100644 index 18deb825c1c..00000000000 --- a/frontend/src/metabase/containers/AddToDashSelectDashModal/AddToDashSelectDashModal.styled.tsx +++ /dev/null @@ -1,10 +0,0 @@ -import styled from "@emotion/styled"; - -import { color } from "metabase/lib/colors"; - -export const LinkContent = styled.div` - display: flex; - align-items: center; - padding: 1rem 0; - color: ${color("text-brand")}; -`; diff --git a/frontend/src/metabase/containers/CollectionPicker.jsx b/frontend/src/metabase/containers/CollectionPicker.jsx deleted file mode 100644 index f162109f996..00000000000 --- a/frontend/src/metabase/containers/CollectionPicker.jsx +++ /dev/null @@ -1,32 +0,0 @@ -import PropTypes from "prop-types"; -import { useState } from "react"; - -import { CreateCollectionOnTheGoButton } from "metabase/containers/CreateCollectionOnTheGo"; - -import ItemPicker from "./ItemPicker"; - -const CollectionPicker = ({ value, onChange, ...props }) => { - const [openCollectionId, setOpenCollectionId] = useState("root"); - return ( - <ItemPicker - {...props} - value={ - value === undefined ? undefined : { model: "collection", id: value } - } - onChange={collection => onChange(collection ? collection.id : undefined)} - models={["collection"]} - onOpenCollectionChange={id => setOpenCollectionId(id)} - > - <CreateCollectionOnTheGoButton openCollectionId={openCollectionId} /> - </ItemPicker> - ); -}; - -CollectionPicker.propTypes = { - // a collection ID or null (for "root" collection), or undefined if none selected - value: PropTypes.number, - // callback that takes a collection ID or null (for "root" collection) - onChange: PropTypes.func.isRequired, -}; - -export default CollectionPicker; diff --git a/frontend/src/metabase/containers/CollectionSelect.jsx b/frontend/src/metabase/containers/CollectionSelect.jsx deleted file mode 100644 index 67a467c9ac4..00000000000 --- a/frontend/src/metabase/containers/CollectionSelect.jsx +++ /dev/null @@ -1,24 +0,0 @@ -import Collection from "metabase/entities/collections"; - -import CollectionName from "./CollectionName"; -import CollectionPicker from "./CollectionPicker"; -import ItemSelect from "./ItemSelect"; - -const CollectionSelect = ItemSelect( - CollectionPicker, - CollectionName, - "collection", -); - -/** - * When suggesting an initial collection, - * we need to check a user has `write` access to it. - * For that, collection objects have to be present in Redux store, - * so we can retrieve a collection by ID and check the `can_write` flag. - * - * This component is wrapped with @Collection.loadList - * to ensure collection are fetched and permissions can be checked. - */ -export default Collection.loadList({ - loadingAndErrorWrapper: false, -})(CollectionSelect); diff --git a/frontend/src/metabase/containers/CreateCollectionOnTheGo.styled.tsx b/frontend/src/metabase/containers/CreateCollectionOnTheGo.styled.tsx deleted file mode 100644 index dbde14a02e3..00000000000 --- a/frontend/src/metabase/containers/CreateCollectionOnTheGo.styled.tsx +++ /dev/null @@ -1,7 +0,0 @@ -import styled from "@emotion/styled"; - -import Button from "metabase/core/components/Button"; - -export const NewCollectionButton = styled(Button)` - margin-top: 0.5rem; -`; diff --git a/frontend/src/metabase/containers/CreateCollectionOnTheGo.tsx b/frontend/src/metabase/containers/CreateCollectionOnTheGo.tsx deleted file mode 100644 index f3b58bf7648..00000000000 --- a/frontend/src/metabase/containers/CreateCollectionOnTheGo.tsx +++ /dev/null @@ -1,91 +0,0 @@ -import type { FormikValues } from "formik"; -import { useFormikContext } from "formik"; -import type { ReactElement } from "react"; -import { useState, useCallback, createContext, useContext } from "react"; -import { t } from "ttag"; - -import CreateCollectionModal from "metabase/collections/containers/CreateCollectionModal"; -import type { Collection, CollectionId } from "metabase-types/api"; - -import { NewCollectionButton } from "./CreateCollectionOnTheGo.styled"; -import type { FilterItemsInPersonalCollection } from "./ItemPicker"; - -interface Values extends FormikValues { - collection_id: CollectionId; -} - -interface State { - enabled?: boolean; - resumedValues?: Values; - openCollectionId?: CollectionId; - filterPersonalCollections?: FilterItemsInPersonalCollection; -} - -const Context = createContext<{ - canCreateNew?: boolean; - updateState?: (newState: State) => void; -}>({}); - -export function CreateCollectionOnTheGo({ - children, -}: { - children: (props: { resumedValues?: Values }) => ReactElement; -}) { - const [state, setState] = useState<State>({}); - const updateState = useCallback( - (newState: State) => setState({ ...state, ...newState }), - [state, setState], - ); - const { - enabled, - resumedValues, - openCollectionId, - filterPersonalCollections, - } = state; - return enabled ? ( - <CreateCollectionModal - collectionId={openCollectionId} - onClose={() => updateState({ enabled: false })} - onCreate={(collection: Collection) => { - updateState({ - enabled: false, - resumedValues: { ...resumedValues, collection_id: collection.id }, - }); - }} - filterPersonalCollections={filterPersonalCollections} - /> - ) : ( - <Context.Provider value={{ canCreateNew: true, updateState }}> - {children({ resumedValues })} - </Context.Provider> - ); -} - -interface CreateCollectionOnTheGoButtonProps { - openCollectionId?: CollectionId; - filterPersonalCollections?: FilterItemsInPersonalCollection; -} - -export function CreateCollectionOnTheGoButton({ - openCollectionId, - filterPersonalCollections, -}: CreateCollectionOnTheGoButtonProps) { - const { canCreateNew, updateState } = useContext(Context); - const formik = useFormikContext<Values>(); - return canCreateNew && formik ? ( - <NewCollectionButton - light - icon="add" - onClick={() => - updateState?.({ - enabled: true, - resumedValues: formik.values, - openCollectionId, - filterPersonalCollections, - }) - } - > - {t`New collection`} - </NewCollectionButton> - ) : null; -} diff --git a/frontend/src/metabase/containers/DashboardPicker.tsx b/frontend/src/metabase/containers/DashboardPicker.tsx deleted file mode 100644 index dec07e8332d..00000000000 --- a/frontend/src/metabase/containers/DashboardPicker.tsx +++ /dev/null @@ -1,32 +0,0 @@ -import type { CollectionId, DashboardId } from "metabase-types/api"; - -import ItemPicker from "./ItemPicker"; -import type { ItemPickerProps } from "./ItemPicker/ItemPicker"; - -export interface DashboardPickerProps - extends Pick< - ItemPickerProps<DashboardId>, - "filterPersonalCollections" | "onOpenCollectionChange" - > { - value?: DashboardId; - onChange: (dashboardId: DashboardId) => void; - collectionId?: CollectionId; -} - -const DashboardPicker = ({ - value, - onChange, - collectionId, - ...props -}: DashboardPickerProps) => ( - <ItemPicker - {...props} - initialOpenCollectionId={collectionId} - value={value === undefined ? undefined : { model: "dashboard", id: value }} - onChange={dashboard => onChange(dashboard.id)} - models={["dashboard"]} - /> -); - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default DashboardPicker; diff --git a/frontend/src/metabase/containers/ItemPicker/Item.styled.tsx b/frontend/src/metabase/containers/ItemPicker/Item.styled.tsx deleted file mode 100644 index 1a8f7c80dab..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/Item.styled.tsx +++ /dev/null @@ -1,82 +0,0 @@ -import { css } from "@emotion/react"; -import styled from "@emotion/styled"; - -import IconButtonWrapper from "metabase/components/IconButtonWrapper"; -import { color } from "metabase/lib/colors"; -import { Icon } from "metabase/ui"; - -export interface ItemRootProps { - canSelect: boolean; - isSelected: boolean; - hasChildren?: boolean; -} - -export const ItemIcon = styled(Icon)``; - -export const ItemRoot = styled.div<ItemRootProps>` - margin-top: 0.5rem; - padding: 0.5rem; - border-radius: 0.5rem; - - ${({ isSelected }) => - isSelected && - css` - color: ${color("white")}; - background-color: ${color("brand")}; - - & ${ExpandButton} { - color: ${color("white")}; - } - `} - - ${({ canSelect, hasChildren }) => - (canSelect || hasChildren) && - css` - cursor: pointer; - - &:hover { - color: ${color("white")}; - background-color: ${color("brand")}; - - & ${ExpandButton} { - /** - * If the item can't be selected, show the ExpandButton's hovered - * state to indicate that the ExapndButton's click handler will be - * called if the user clicks on the item. - */ - color: ${canSelect ? color("white") : color("brand")}; - background-color: ${canSelect ? color("brand") : color("white")}; - - &:hover { - color: ${color("brand")}; - & ${ItemIcon} { - color: ${color("brand")}; - } - background-color: ${color("white")}; - } - } - } - `} -`; - -export const ItemContent = styled.div` - display: flex; - align-items: center; -`; - -export const ItemTitle = styled.h4` - margin-left: 0.5rem; - margin-right: 0.5rem; -`; - -export const ExpandButton = styled(IconButtonWrapper)<{ canSelect: boolean }>` - padding: 0.5rem; - margin-left: auto; - - color: ${color("text-light")}; - border: 1px solid ${color("border")}; -`; - -ExpandButton.defaultProps = { - circle: true, -}; diff --git a/frontend/src/metabase/containers/ItemPicker/Item.tsx b/frontend/src/metabase/containers/ItemPicker/Item.tsx deleted file mode 100644 index 593cdb72f5c..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/Item.tsx +++ /dev/null @@ -1,82 +0,0 @@ -import { useCallback, useMemo } from "react"; -import _ from "underscore"; - -import type { IconName, IconProps } from "metabase/ui"; -import { Icon } from "metabase/ui"; - -import { ItemRoot, ItemContent, ItemTitle, ExpandButton } from "./Item.styled"; -import type { PickerItem } from "./types"; - -interface Props<TId> { - item: PickerItem<TId>; - name: string; - icon: IconName | IconProps; - selected: boolean; - canSelect: boolean; - hasChildren?: boolean; - onChange: (item: PickerItem<TId>) => void; - onChangeOpenCollectionId?: (id: TId) => void; -} - -function Item<TId>({ - item, - name, - icon, - selected, - canSelect, - hasChildren, - onChange, - onChangeOpenCollectionId, -}: Props<TId>) { - const handleClick = useMemo(() => { - if (canSelect) { - return () => onChange(item); - } - if (hasChildren) { - return () => onChangeOpenCollectionId?.(item.id); - } - return; - }, [item, canSelect, hasChildren, onChange, onChangeOpenCollectionId]); - - const handleExpand = useCallback( - (event: React.MouseEvent) => { - event.stopPropagation(); - onChangeOpenCollectionId?.(item.id); - }, - [item, onChangeOpenCollectionId], - ); - - const iconProps = useMemo( - () => (_.isObject(icon) ? icon : { name: icon }), - [icon], - ); - - return ( - <ItemRoot - onClick={handleClick} - canSelect={canSelect} - isSelected={selected} - hasChildren={hasChildren} - data-testid="item-picker-item" - aria-selected={selected} - role="option" - > - <ItemContent> - <Icon {...iconProps} /> - <ItemTitle>{name}</ItemTitle> - {hasChildren && ( - <ExpandButton - canSelect={canSelect} - onClick={handleExpand} - data-testid="expand-btn" - > - <Icon name="chevronright" /> - </ExpandButton> - )} - </ItemContent> - </ItemRoot> - ); -} - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default Item; diff --git a/frontend/src/metabase/containers/ItemPicker/ItemPicker.styled.tsx b/frontend/src/metabase/containers/ItemPicker/ItemPicker.styled.tsx deleted file mode 100644 index 4a023b3a785..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/ItemPicker.styled.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import { css } from "@emotion/react"; -import styled from "@emotion/styled"; - -import IconButtonWrapper from "metabase/components/IconButtonWrapper"; -import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; -import { color } from "metabase/lib/colors"; - -export const ScrollAwareLoadingAndErrorWrapper = styled( - LoadingAndErrorWrapper, -)<{ hasScroll?: boolean }>` - ${props => - props.hasScroll && - css` - overflow-y: auto; - `} -`; - -export const ItemPickerRoot = styled.div` - overflow-y: auto; -`; - -export const ItemPickerHeader = styled.div` - display: flex; - align-items: center; - - margin-bottom: 1rem; - padding-bottom: 0.5rem; - - border-bottom: 1px solid ${color("border")}; -`; - -export const ItemPickerList = styled.div` - overflow-y: auto; -`; - -export const SearchInput = styled.input` - flex: 1 0 auto; - border-radius: 8px; -`; - -export const SearchToggle = styled(IconButtonWrapper)` - margin-left: auto; - padding-left: 1rem; - - color: ${color("text-light")}; - - &:hover { - color: ${color("text-medium")}; - } -`; diff --git a/frontend/src/metabase/containers/ItemPicker/ItemPicker.tsx b/frontend/src/metabase/containers/ItemPicker/ItemPicker.tsx deleted file mode 100644 index 74a5a73fdd8..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/ItemPicker.tsx +++ /dev/null @@ -1,293 +0,0 @@ -import type * as React from "react"; -import { useCallback, useMemo, useState } from "react"; -import { connect } from "react-redux"; -import _ from "underscore"; - -import { - isPersonalCollection, - isPublicCollection, - isRootCollection, -} from "metabase/collections/utils"; -import Collections from "metabase/entities/collections"; -import { entityListLoader } from "metabase/entities/containers/EntityListLoader"; -import { entityObjectLoader } from "metabase/entities/containers/EntityObjectLoader"; -import { getCrumbs } from "metabase/lib/collections"; -import type { IconProps } from "metabase/ui"; -import type { Collection, CollectionId } from "metabase-types/api"; -import type { State } from "metabase-types/store"; - -import { ScrollAwareLoadingAndErrorWrapper } from "./ItemPicker.styled"; -import ItemPickerView from "./ItemPickerView"; -import type { - CollectionPickerItem, - FilterItemsInPersonalCollection, - PickerItem, - PickerModel, - PickerValue, - SearchQuery, -} from "./types"; - -interface OwnProps<TId> { - value?: PickerValue<TId>; - models: PickerModel[]; - entity?: typeof Collections; // collections/snippets entity - showSearch?: boolean; - showScroll?: boolean; - filterPersonalCollections?: FilterItemsInPersonalCollection; - className?: string; - style?: React.CSSProperties; - onChange: (value: PickerValue<TId>) => void; - initialOpenCollectionId?: CollectionId; - collectionFilter?: (collection: Collection) => boolean; - onOpenCollectionChange?: (collectionId: CollectionId) => void; - children?: React.ReactNode; -} - -interface StateProps { - collectionsById: Record<CollectionId, Collection>; - getCollectionIcon: (collection: Collection) => IconProps; -} - -export type ItemPickerProps<TId> = OwnProps<TId> & StateProps; - -function canWriteToCollectionOrChildren(collection: Collection) { - return ( - collection.can_write || - collection.children?.some(canWriteToCollectionOrChildren) - ); -} - -function mapStateToProps<TId>(state: State, props: OwnProps<TId>) { - const entity = props.entity || Collections; - return { - collectionsById: entity.selectors.getExpandedCollectionsById(state, props), - getCollectionIcon: entity.objectSelectors.getIcon, - }; -} - -function getEntityLoaderType<TId>(state: State, props: OwnProps<TId>) { - return props.entity?.name ?? "collections"; -} - -function getItemId<TId>(item: PickerItem<TId> | PickerValue<TId>) { - if (!item) { - return; - } - if (item.model === "collection") { - return item.id === null ? "root" : item.id; - } - return item.id; -} - -function ItemPicker<TId>({ - value, - models, - collectionsById, - className, - showSearch = true, - showScroll = true, - filterPersonalCollections, - style, - onChange, - getCollectionIcon, - initialOpenCollectionId = "root", - onOpenCollectionChange, - children, -}: ItemPickerProps<TId>) { - const [openCollectionId, setOpenCollectionId] = useState<CollectionId>( - initialOpenCollectionId, - ); - const [searchString, setSearchString] = useState(""); - - const isPickingNotCollection = models.some(model => model !== "collection"); - - const openCollection = collectionsById[openCollectionId]; - const isOpenCollectionInPersonalCollection = openCollection?.is_personal; - const showItems = Boolean( - searchString || - filterPersonalCollections !== "only" || - isOpenCollectionInPersonalCollection, - ); - - const collections = useMemo(() => { - let list = openCollection?.children || []; - - // show root in itself if we can pick it - if ( - openCollection && - isRootCollection(openCollection) && - models.includes("collection") - ) { - list = [openCollection, ...list]; - } - - const collectionItems = list - .filter(canWriteToCollectionOrChildren) - .filter(getCollectionFilter(filterPersonalCollections)) - .map(collection => ({ - ...collection, - model: "collection", - })); - - return collectionItems as CollectionPickerItem<TId>[]; - }, [openCollection, models, filterPersonalCollections]); - - const crumbs = useMemo( - () => - getCrumbs(openCollection, collectionsById, id => { - setOpenCollectionId(id); - onOpenCollectionChange?.(id); - }), - [openCollection, collectionsById, onOpenCollectionChange], - ); - - const searchQuery = useMemo(() => { - const query: SearchQuery = {}; - - if (searchString) { - query.q = searchString; - if (filterPersonalCollections) { - query.filter_items_in_personal_collection = filterPersonalCollections; - } - } else { - query.collection = openCollectionId; - } - - query.models = models; - - return query; - }, [searchString, models, filterPersonalCollections, openCollectionId]); - - const checkIsItemSelected = useCallback( - (item: PickerItem<TId>) => { - if (!value || !item) { - return false; - } - const isSameModel = item.model === value.model || models.length === 1; - return isSameModel && getItemId(item) === getItemId(value); - }, - [value, models], - ); - - const checkCollectionMaybeHasChildren = useCallback( - (collection: CollectionPickerItem<TId>) => { - if (isPickingNotCollection) { - // Non-collection models (e.g. questions, dashboards) - // are loaded on-demand so we don't know ahead of time - // if they have children, so we have to assume they do - return true; - } - - if (isRootCollection(collection)) { - // Skip root as we don't show root's sub-collections alongside it - return false; - } - - return ( - Array.isArray(collection.children) && collection.children.length > 0 - ); - }, - [isPickingNotCollection], - ); - - const checkHasWritePermissionForItem = useCallback( - (item: PickerItem<TId>) => { - // if user is selecting a collection, they must have a `write` access to it - if (models.includes("collection") && item.model === "collection") { - return item.can_write; - } - - // if user is selecting something else (e.g. dashboard), - // they must have `write` access to a collection item belongs to - const collection = item.collection_id - ? collectionsById[item.collection_id] - : collectionsById["root"]; - return collection?.can_write; - }, - [models, collectionsById], - ); - - const handleChange = useCallback( - (item: PickerItem<TId>) => { - if ( - item.model === "collection" && - isRootCollection(item as unknown as Collection) - ) { - onChange({ - id: null, - model: "collection", - } as unknown as PickerItem<TId>); - } else { - onChange(item); - } - }, - [onChange], - ); - - const handleCollectionOpen = useCallback( - (collectionId: CollectionId) => { - setOpenCollectionId(collectionId); - onOpenCollectionChange?.(collectionId); - }, - [onOpenCollectionChange], - ); - - return ( - <ScrollAwareLoadingAndErrorWrapper - loading={!collectionsById} - hasScroll={showScroll} - > - <ItemPickerView - className={className} - models={models} - collections={collections} - searchString={searchString} - searchQuery={searchQuery} - showSearch={showSearch} - crumbs={crumbs} - onChange={handleChange} - onSearchStringChange={setSearchString} - onOpenCollectionChange={handleCollectionOpen} - checkCollectionMaybeHasChildren={checkCollectionMaybeHasChildren} - checkIsItemSelected={checkIsItemSelected} - checkHasWritePermissionForItem={checkHasWritePermissionForItem} - getCollectionIcon={getCollectionIcon} - style={style} - allowFetch={ - // "personal" is a fake collection for admins that contains all other user's collections - openCollectionId !== "personal" && showItems - } - > - {children} - </ItemPickerView> - </ScrollAwareLoadingAndErrorWrapper> - ); -} - -function getCollectionFilter( - filterPersonalCollections?: FilterItemsInPersonalCollection, -) { - if (filterPersonalCollections === "only") { - return isPersonalCollection; - } - - if (filterPersonalCollections === "exclude") { - return isPublicCollection; - } - - return _.identity; -} - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default _.compose( - entityObjectLoader({ - id: "root", - entityType: getEntityLoaderType, - loadingAndErrorWrapper: false, - }), - entityListLoader({ - entityType: getEntityLoaderType, - loadingAndErrorWrapper: false, - }), - connect(mapStateToProps), -)(ItemPicker) as <TId>(props: OwnProps<TId>) => JSX.Element; diff --git a/frontend/src/metabase/containers/ItemPicker/ItemPicker.unit.spec.js b/frontend/src/metabase/containers/ItemPicker/ItemPicker.unit.spec.js deleted file mode 100644 index f45369bf6ba..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/ItemPicker.unit.spec.js +++ /dev/null @@ -1,383 +0,0 @@ -import userEvent from "@testing-library/user-event"; - -import { - setupCollectionsEndpoints, - setupDashboardCollectionItemsEndpoint, - setupSearchEndpoints, -} from "__support__/server-mocks"; -import { - renderWithProviders, - screen, - waitForLoaderToBeRemoved, - within, -} from "__support__/ui"; -import { isPersonalCollectionOrChild } from "metabase/collections/utils"; -import { ROOT_COLLECTION } from "metabase/entities/collections"; -import SnippetCollections from "metabase/entities/snippet-collections"; -import { - createMockCollection, - createMockDashboard, - createMockUser, -} from "metabase-types/api/mocks"; - -import ItemPicker from "./ItemPicker"; -import { - getItemPickerHeader, - getItemPickerList, - openCollectionWait, -} from "./test-utils"; - -const CURRENT_USER = createMockUser({ - id: 1, - personal_collection_id: 100, - is_superuser: true, -}); - -const COLLECTION = { - ROOT: createMockCollection({ - ...ROOT_COLLECTION, - can_write: true, - }), - PERSONAL: createMockCollection({ - id: CURRENT_USER.personal_collection_id, - name: "My personal collection", - personal_owner_id: CURRENT_USER.id, - can_write: true, - }), - REGULAR: createMockCollection({ - id: 1, - name: "Regular collection", - can_write: true, - }), - REGULAR_2: createMockCollection({ - id: 6, - name: "Regular collection 2", - can_write: true, - }), - READ_ONLY: createMockCollection({ - id: 2, - name: "Read only collection", - can_write: false, - }), -}; - -COLLECTION.REGULAR_CHILD = createMockCollection({ - id: 3, - name: "Regular collection's child", - location: `/${COLLECTION.REGULAR.id}/`, - can_write: true, -}); - -const COLLECTION_READ_ONLY_CHILD_WRITABLE = createMockCollection({ - id: 4, - name: "Read-only collection's child (writable)", - location: `/${COLLECTION.READ_ONLY.id}/`, - can_write: true, -}); - -const COLLECTION_OTHER_USERS = createMockCollection({ - id: 5, - name: "John Lennon's personal collection", - personal_owner_id: CURRENT_USER.id + 1, - can_write: true, -}); - -const DASHBOARD = { - REGULAR: createMockDashboard({ - id: 1, - name: "Regular dashboard", - model: "dashboard", - }), - REGULAR_CHILD: createMockDashboard({ - id: 2, - name: "Regular dashboard (nested)", - model: "dashboard", - collection_id: COLLECTION.REGULAR.id, - }), - PERSONAL_CHILD: createMockDashboard({ - id: 3, - name: "Personal dashboard", - model: "dashboard", - collection_id: COLLECTION.PERSONAL.id, - }), -}; - -async function setup({ - models = ["dashboard"], - collections = Object.values(COLLECTION), - rootCollection = COLLECTION.ROOT, - query, - ...props -} = {}) { - if (models.includes("dashboard")) { - setupDashboardCollectionItemsEndpoint(Object.values(DASHBOARD)); - } - - setupCollectionsEndpoints({ collections, rootCollection }); - setupSearchEndpoints([ - DASHBOARD.REGULAR, - DASHBOARD.REGULAR_CHILD, - DASHBOARD.PERSONAL_CHILD, - ]); - - const onChange = jest.fn(); - - renderWithProviders( - <ItemPicker models={models} query={query} onChange={onChange} {...props} />, - { - storeInitialState: { - currentUser: CURRENT_USER, - }, - }, - ); - - await waitForLoaderToBeRemoved(); - - return { onChange }; -} - -describe("ItemPicker", () => { - it("displays items from the root collection by default", async () => { - await setup(); - - // Breadcrumbs - expect( - within(getItemPickerHeader()).getByText(/Our analytics/i), - ).toBeInTheDocument(); - - // Content - expect(screen.getByText(DASHBOARD.REGULAR.name)).toBeInTheDocument(); - expect(screen.getByText(COLLECTION.REGULAR.name)).toBeInTheDocument(); - expect(screen.getByText(COLLECTION.REGULAR_2.name)).toBeInTheDocument(); - expect(screen.getByText(COLLECTION.PERSONAL.name)).toBeInTheDocument(); - expect(screen.queryAllByTestId("item-picker-item")).toHaveLength(4); - }); - - it("does not display read-only collections", async () => { - await setup(); - expect( - screen.queryByText(COLLECTION.READ_ONLY.name), - ).not.toBeInTheDocument(); - }); - - it("displays read-only collections if they have writable children", async () => { - await setup({ - collections: [ - ...Object.values(COLLECTION), - COLLECTION_READ_ONLY_CHILD_WRITABLE, - ], - }); - expect(screen.getByText(COLLECTION.READ_ONLY.name)).toBeInTheDocument(); - }); - - it("can open nested collection", async () => { - await setup(); - - await openCollectionWait(COLLECTION.REGULAR.name); - - const header = within(getItemPickerHeader()); - const list = within(getItemPickerList()); - - // Breadcrumbs - expect(header.getByText(/Our analytics/i)).toBeInTheDocument(); - expect(header.getByText(COLLECTION.REGULAR.name)).toBeInTheDocument(); - - // Content - expect(list.getByText(COLLECTION.REGULAR_CHILD.name)).toBeInTheDocument(); - expect(list.getByText(DASHBOARD.REGULAR_CHILD.name)).toBeInTheDocument(); - expect(list.getAllByTestId("item-picker-item")).toHaveLength(2); - }); - - it("can navigate back from a currently open nested collection", async () => { - await setup(); - await openCollectionWait(COLLECTION.REGULAR.name); - let header = within(getItemPickerHeader()); - - await userEvent.click(header.getByText(/Our analytics/i)); - - header = within(getItemPickerHeader()); - const list = within(getItemPickerList()); - - expect(header.queryByText(COLLECTION.REGULAR.name)).not.toBeInTheDocument(); - - expect(list.getByText(DASHBOARD.REGULAR.name)).toBeInTheDocument(); - expect(list.getByText(COLLECTION.REGULAR.name)).toBeInTheDocument(); - expect(list.getByText(COLLECTION.REGULAR_2.name)).toBeInTheDocument(); - expect(list.getByText(COLLECTION.PERSONAL.name)).toBeInTheDocument(); - expect(list.getAllByTestId("item-picker-item")).toHaveLength(4); - }); - - it("calls onChange when selecting an item", async () => { - const { onChange } = await setup(); - - await userEvent.click(screen.getByText(DASHBOARD.REGULAR.name)); - - expect(onChange).toHaveBeenCalledWith( - expect.objectContaining(DASHBOARD.REGULAR), - ); - expect(onChange).toHaveBeenCalledTimes(1); - }); - - it("doesn't call onChange if it's not a collection picker", async () => { - const { onChange } = await setup(); - await userEvent.click(screen.getByText(COLLECTION.REGULAR.name)); - expect(onChange).not.toHaveBeenCalled(); - }); - - it("groups personal collections into single folder if there are more than one", async () => { - await setup({ - collections: [...Object.values(COLLECTION), COLLECTION_OTHER_USERS], - }); - - await userEvent.click(screen.getByText(/All personal collections/i)); - - const list = within(getItemPickerList()); - expect(list.getByText(COLLECTION_OTHER_USERS.name)).toBeInTheDocument(); - expect(list.getByText(COLLECTION.PERSONAL.name)).toBeInTheDocument(); - expect(list.getAllByTestId("item-picker-item")).toHaveLength(2); - }); - - it("preselects value in the correspondent collection", async () => { - await setup({ - initialOpenCollectionId: DASHBOARD.REGULAR_CHILD.collection_id, - value: { - model: "dashboard", - id: DASHBOARD.REGULAR_CHILD.id, - }, - }); - - const header = within(getItemPickerHeader()); - - // nested collection - expect(header.getByText(COLLECTION.ROOT.name)).toBeInTheDocument(); - expect(header.getByText(COLLECTION.REGULAR.name)).toBeInTheDocument(); - - const list = within(getItemPickerList()); - expect(list.getByText(DASHBOARD.REGULAR_CHILD.name)).toBeInTheDocument(); - }); - - describe("preserves order of collections coming from API endpoint", () => { - it("[personal, regular, regular 2]", async () => { - const collections = [ - COLLECTION.PERSONAL, - COLLECTION.REGULAR, - COLLECTION.REGULAR_2, - ]; - - await setup({ collections }); - - const items = screen.getAllByTestId("item-picker-item"); - - expect(items.length).toBe(collections.length); - expect(items[0]).toHaveTextContent(collections[0].name); - expect(items[1]).toHaveTextContent(collections[1].name); - expect(items[2]).toHaveTextContent(collections[2].name); - }); - - it("[personal, regular 2, regular]", async () => { - const collections = [ - COLLECTION.PERSONAL, - COLLECTION.REGULAR_2, - COLLECTION.REGULAR, - ]; - - await setup({ collections }); - - const items = screen.getAllByTestId("item-picker-item"); - - expect(items.length).toBe(collections.length); - expect(items[0]).toHaveTextContent(collections[0].name); - expect(items[1]).toHaveTextContent(collections[1].name); - expect(items[2]).toHaveTextContent(collections[2].name); - }); - - it("always shows personal collection first", async () => { - const collections = [ - COLLECTION.REGULAR_2, - COLLECTION.REGULAR, - COLLECTION.PERSONAL, - ]; - - await setup({ collections }); - - const items = screen.getAllByTestId("item-picker-item"); - - expect(items.length).toBe(collections.length); - expect(items[0]).toHaveTextContent(COLLECTION.PERSONAL.name); - expect(items[1]).toHaveTextContent(COLLECTION.REGULAR_2.name); - expect(items[2]).toHaveTextContent(COLLECTION.REGULAR.name); - }); - - it("should filter collections", async () => { - await setup({ - query: "foo", - collectionFilter: (collection, _index, allCollections) => - !isPersonalCollectionOrChild(collection, allCollections), - }); - - expect(screen.queryByText(/personal/i)).not.toBeInTheDocument(); - }); - - it("should show search results", async () => { - await setup(); - - await userEvent.click(screen.getByRole("img", { name: /search/ })); - await userEvent.type(screen.getByPlaceholderText("Search"), "das{enter}"); - - expect( - await screen.findByText(/^regular dashboard$/i), - ).toBeInTheDocument(); - expect(await screen.findByText(/nested/i)).toBeInTheDocument(); - expect(await screen.findByText(/personal/i)).toBeInTheDocument(); - }); - - it("should not show items of filtered collections when searching", async () => { - await setup({ - collectionFilter: (collection, _index, allCollections) => - !isPersonalCollectionOrChild(collection, allCollections), - }); - - await userEvent.click(screen.getByRole("img", { name: /search/ })); - await userEvent.type(screen.getByPlaceholderText("Search"), "das{enter}"); - - expect( - await screen.findByText(/^regular dashboard$/i), - ).toBeInTheDocument(); - expect(screen.queryByText(/personal/i)).not.toBeInTheDocument(); - }); - }); - - describe("preserves order of snippet collections coming from API endpoint", () => { - it("[regular, regular 2]", async () => { - const collections = [COLLECTION.REGULAR, COLLECTION.REGULAR_2]; - - await setup({ - collections, - entity: SnippetCollections, - query: { namespace: "snippets" }, - }); - - const items = screen.getAllByTestId("item-picker-item"); - - expect(items.length).toBe(collections.length); - expect(items[0]).toHaveTextContent(collections[0].name); - expect(items[1]).toHaveTextContent(collections[1].name); - }); - - it("[regular 2, regular]", async () => { - const collections = [COLLECTION.REGULAR_2, COLLECTION.REGULAR]; - - await setup({ - collections, - entity: SnippetCollections, - query: { namespace: "snippets" }, - }); - - const items = screen.getAllByTestId("item-picker-item"); - - expect(items.length).toBe(collections.length); - expect(items[0]).toHaveTextContent(collections[0].name); - expect(items[1]).toHaveTextContent(collections[1].name); - }); - }); -}); diff --git a/frontend/src/metabase/containers/ItemPicker/ItemPickerView.tsx b/frontend/src/metabase/containers/ItemPicker/ItemPickerView.tsx deleted file mode 100644 index 77ccbee7dae..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/ItemPickerView.tsx +++ /dev/null @@ -1,226 +0,0 @@ -import type * as React from "react"; -import { useCallback, useState } from "react"; -import { t } from "ttag"; - -import Breadcrumbs from "metabase/components/Breadcrumbs"; -import CS from "metabase/css/core/index.css"; -import Search from "metabase/entities/search"; -import type { IconProps } from "metabase/ui"; -import { Icon } from "metabase/ui"; -import type { Collection } from "metabase-types/api"; - -import Item from "./Item"; -import { - ItemPickerRoot, - ItemPickerHeader, - ItemPickerList, - SearchInput, - SearchToggle, -} from "./ItemPicker.styled"; -import type { - CollectionPickerItem, - PickerItem, - PickerModel, - SearchQuery, -} from "./types"; - -interface SearchEntityListLoaderProps<TId> { - list: PickerItem<TId>[]; -} - -interface Props<TId> { - models: PickerModel[]; - collections: CollectionPickerItem<TId>[]; - searchString: string; - searchQuery: SearchQuery; - showSearch?: boolean; - allowFetch?: boolean; - crumbs: any[]; - className?: string; - style?: React.CSSProperties; - onChange: (item: PickerItem<TId>) => void; - onSearchStringChange: (searchString: string) => void; - onOpenCollectionChange: ( - collectionId: CollectionPickerItem<TId>["id"], - ) => void; - checkCollectionMaybeHasChildren: ( - collection: CollectionPickerItem<TId>, - ) => boolean; - checkIsItemSelected: (item: PickerItem<TId>) => boolean; - checkHasWritePermissionForItem: (item: PickerItem<TId>) => boolean; - getCollectionIcon: (collection: Collection) => IconProps; - children: React.ReactNode; -} - -function ItemPickerView<TId>({ - collections, - models, - searchString, - searchQuery, - showSearch = true, - allowFetch = true, - crumbs, - className, - style, - onChange, - onSearchStringChange, - onOpenCollectionChange, - checkCollectionMaybeHasChildren, - checkIsItemSelected, - checkHasWritePermissionForItem, - getCollectionIcon, - children, -}: Props<TId>) { - const [isSearchEnabled, setIsSearchEnabled] = useState(false); - - const isPickingNotCollection = models.some(model => model !== "collection"); - const canFetch = (isPickingNotCollection || searchString) && allowFetch; - - const handleSearchInputKeyPress = useCallback( - (e: React.KeyboardEvent<HTMLInputElement>) => { - if (e.key === "Enter") { - onSearchStringChange(e.currentTarget.value); - } - }, - [onSearchStringChange], - ); - - const handleOpenSearch = useCallback(() => { - setIsSearchEnabled(true); - }, []); - - const handleCloseSearch = useCallback(() => { - setIsSearchEnabled(false); - onSearchStringChange(""); - }, [onSearchStringChange]); - - const renderHeader = useCallback(() => { - if (isSearchEnabled) { - return ( - <ItemPickerHeader data-testid="item-picker-header"> - <SearchInput - type="search" - className={CS.input} - placeholder={t`Search`} - autoFocus - onKeyPress={handleSearchInputKeyPress} - /> - <SearchToggle onClick={handleCloseSearch}> - <Icon name="close" /> - </SearchToggle> - </ItemPickerHeader> - ); - } - - return ( - <ItemPickerHeader data-testid="item-picker-header"> - <Breadcrumbs crumbs={crumbs} /> - {showSearch && ( - <SearchToggle onClick={handleOpenSearch} aria-label={t`Search`}> - <Icon name="search" /> - </SearchToggle> - )} - </ItemPickerHeader> - ); - }, [ - isSearchEnabled, - crumbs, - showSearch, - handleOpenSearch, - handleCloseSearch, - handleSearchInputKeyPress, - ]); - - const renderCollectionListItem = useCallback( - (collection: CollectionPickerItem<TId>) => { - const hasChildren = checkCollectionMaybeHasChildren(collection); - - // NOTE: this assumes the only reason you'd be selecting a collection is to modify it in some way - const canSelect = models.includes("collection") && collection.can_write; - - const icon = getCollectionIcon(collection); - - if (canSelect || hasChildren) { - return ( - <Item - key={`collection-${collection.id}`} - item={collection} - name={collection.name} - icon={icon} - selected={canSelect && checkIsItemSelected(collection)} - canSelect={canSelect} - hasChildren={hasChildren} - onChange={onChange} - onChangeOpenCollectionId={onOpenCollectionChange} - /> - ); - } - - return null; - }, - [ - models, - onChange, - getCollectionIcon, - onOpenCollectionChange, - checkIsItemSelected, - checkCollectionMaybeHasChildren, - ], - ); - - const renderCollectionContentListItem = useCallback( - (item: PickerItem<TId>) => { - const hasPermission = checkHasWritePermissionForItem(item); - - if ( - hasPermission && - // only include desired models (TODO: ideally the endpoint would handle this) - models.includes(item.model) && - // remove collections unless we're searching - // (so a user can navigate through collections) - (item.model !== "collection" || searchString) - ) { - return ( - <Item - key={`${item.id}`} - item={item} - name={item.getName()} - icon={item.getIcon().name} - selected={checkIsItemSelected(item)} - canSelect={hasPermission} - onChange={onChange} - /> - ); - } - - return null; - }, - [ - models, - searchString, - onChange, - checkHasWritePermissionForItem, - checkIsItemSelected, - ], - ); - - return ( - <ItemPickerRoot className={className} style={style}> - {renderHeader()} - <ItemPickerList data-testid="item-picker-list" role="list"> - {!searchString && collections.map(renderCollectionListItem)} - {children} - {canFetch && ( - <Search.ListLoader query={searchQuery} wrapped> - {({ list }: SearchEntityListLoaderProps<TId>) => ( - <div>{list.map(renderCollectionContentListItem)}</div> - )} - </Search.ListLoader> - )} - </ItemPickerList> - </ItemPickerRoot> - ); -} - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default ItemPickerView; diff --git a/frontend/src/metabase/containers/ItemPicker/index.ts b/frontend/src/metabase/containers/ItemPicker/index.ts deleted file mode 100644 index a0c07a4df59..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/index.ts +++ /dev/null @@ -1,3 +0,0 @@ -// eslint-disable-next-line import/no-default-export -- deprecated usage -export { default } from "./ItemPicker"; -export * from "./types"; diff --git a/frontend/src/metabase/containers/ItemPicker/test-utils.js b/frontend/src/metabase/containers/ItemPicker/test-utils.js deleted file mode 100644 index b888ddb3766..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/test-utils.js +++ /dev/null @@ -1,27 +0,0 @@ -import userEvent from "@testing-library/user-event"; - -import { screen, waitForLoaderToBeRemoved, within } from "__support__/ui"; - -export function getItemPickerHeader() { - return screen.getByTestId("item-picker-header"); -} - -export function getItemPickerList() { - return screen.getByTestId("item-picker-list"); -} - -export function queryListItem(itemName) { - return within(getItemPickerList()) - .queryByText(itemName) - .closest("[data-testid=item-picker-item]"); -} - -export async function openCollection(itemName) { - const collectionNode = within(queryListItem(itemName)); - await userEvent.click(collectionNode.getByLabelText("chevronright icon")); -} - -export async function openCollectionWait(itemName) { - await openCollection(itemName); - await waitForLoaderToBeRemoved(); -} diff --git a/frontend/src/metabase/containers/ItemPicker/types.ts b/frontend/src/metabase/containers/ItemPicker/types.ts deleted file mode 100644 index 7991a0377c7..00000000000 --- a/frontend/src/metabase/containers/ItemPicker/types.ts +++ /dev/null @@ -1,29 +0,0 @@ -import type { IconProps } from "metabase/ui"; -import type { Collection } from "metabase-types/api"; - -export type PickerModel = "card" | "collection" | "dataset" | "dashboard"; - -export type PickerValue<TId> = { id: TId; model: PickerModel }; - -export type PickerItem<TId> = { - id: TId; - model: PickerModel; - collection_id: Collection["id"]; - can_write: boolean; - - // Coming from `wrapped` entities - getName: () => string; - getColor: () => string; - getIcon: () => IconProps; -}; - -export type CollectionPickerItem<T> = PickerItem<T> & Collection; - -export type SearchQuery = { - q?: string; - collection?: Collection["id"]; - models?: PickerModel[]; - filter_items_in_personal_collection?: FilterItemsInPersonalCollection; -}; - -export type FilterItemsInPersonalCollection = "only" | "exclude"; diff --git a/frontend/src/metabase/containers/QuestionPicker.jsx b/frontend/src/metabase/containers/QuestionPicker.jsx deleted file mode 100644 index 89231851d51..00000000000 --- a/frontend/src/metabase/containers/QuestionPicker.jsx +++ /dev/null @@ -1,26 +0,0 @@ -/* eslint-disable react/prop-types */ -import PropTypes from "prop-types"; - -import ItemPicker from "./ItemPicker"; - -const QuestionPicker = ({ value, onChange, maxHeight, ...props }) => ( - <ItemPicker - {...props} - // maxHeight is set when rendered in a popover - style={maxHeight != null ? { maxHeight } : {}} - value={value === undefined ? undefined : { model: "card", id: value }} - onChange={question => onChange(question ? question.id : undefined)} - models={["card", "dataset"]} - /> -); - -QuestionPicker.propTypes = { - // a question ID or null - value: PropTypes.number, - // callback that takes a question ID - onChange: PropTypes.func.isRequired, -}; -/** - * @deprecated use metabase/common/components QuestionPicker instead - */ -export default QuestionPicker; diff --git a/frontend/src/metabase/containers/QuestionSelect.tsx b/frontend/src/metabase/containers/QuestionSelect.tsx deleted file mode 100644 index fc5182ca418..00000000000 --- a/frontend/src/metabase/containers/QuestionSelect.tsx +++ /dev/null @@ -1,14 +0,0 @@ -import Question from "metabase/entities/questions"; -import type { CardId } from "metabase-types/api"; - -import ItemSelect from "./ItemSelect"; -import QuestionPicker from "./QuestionPicker"; - -const QuestionName = ({ questionId }: { questionId: CardId }) => ( - <Question.Name id={questionId} /> -); - -const QuestionSelect = ItemSelect(QuestionPicker, QuestionName, "question"); - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default QuestionSelect; diff --git a/frontend/src/metabase/containers/SaveQuestionModal/SaveQuestionModal.tsx b/frontend/src/metabase/containers/SaveQuestionModal/SaveQuestionModal.tsx index d32fe32faf7..a1a17858414 100644 --- a/frontend/src/metabase/containers/SaveQuestionModal/SaveQuestionModal.tsx +++ b/frontend/src/metabase/containers/SaveQuestionModal/SaveQuestionModal.tsx @@ -10,7 +10,6 @@ import { getInstanceAnalyticsCustomCollection, } from "metabase/collections/utils"; import { useCollectionListQuery } from "metabase/common/hooks"; -import { CreateCollectionOnTheGo } from "metabase/containers/CreateCollectionOnTheGo"; import Button from "metabase/core/components/Button"; import FormErrorMessage from "metabase/core/components/FormErrorMessage"; import FormFooter from "metabase/core/components/FormFooter"; @@ -225,84 +224,77 @@ export const SaveQuestionModal = ({ return ( <Modal.Root onClose={onClose} opened={true}> <Modal.Overlay /> - <CreateCollectionOnTheGo> - {({ resumedValues }) => ( - <FormProvider - initialValues={{ ...initialValues, ...resumedValues }} - onSubmit={handleSubmit} - validationSchema={SAVE_QUESTION_SCHEMA} - enableReinitialize - > - {({ values, setValues }) => ( - <Modal.Content p="md" data-testid="save-question-modal"> - <Modal.Header> - <Modal.Title>{title}</Modal.Title> - <Flex align="center" justify="flex-end" gap="sm"> - <PLUGIN_LLM_AUTODESCRIPTION.LLMSuggestQuestionInfo - question={submittableQuestion} - onAccept={nextValues => - setValues({ ...values, ...nextValues }) - } - /> - <Modal.CloseButton /> - </Flex> - </Modal.Header> - <Modal.Body> - <Form> - {showSaveType && ( - <FormRadio - name="saveType" - title={t`Replace or save as new?`} - options={[ - { - name: t`Replace original question, "${originalQuestion?.displayName()}"`, - value: "overwrite", - }, - { name: t`Save as new question`, value: "create" }, - ]} - vertical + <FormProvider + initialValues={{ ...initialValues }} + onSubmit={handleSubmit} + validationSchema={SAVE_QUESTION_SCHEMA} + enableReinitialize + > + {({ values, setValues }) => ( + <Modal.Content p="md" data-testid="save-question-modal"> + <Modal.Header> + <Modal.Title>{title}</Modal.Title> + <Flex align="center" justify="flex-end" gap="sm"> + <PLUGIN_LLM_AUTODESCRIPTION.LLMSuggestQuestionInfo + question={submittableQuestion} + onAccept={nextValues => + setValues({ ...values, ...nextValues }) + } + /> + <Modal.CloseButton /> + </Flex> + </Modal.Header> + <Modal.Body> + <Form> + {showSaveType && ( + <FormRadio + name="saveType" + title={t`Replace or save as new?`} + options={[ + { + name: t`Replace original question, "${originalQuestion?.displayName()}"`, + value: "overwrite", + }, + { name: t`Save as new question`, value: "create" }, + ]} + vertical + /> + )} + <TransitionGroup> + {values.saveType === "create" && ( + <div className={CS.overflowHidden}> + <FormInput + name="name" + title={t`Name`} + placeholder={nameInputPlaceholder} /> - )} - <TransitionGroup> - {values.saveType === "create" && ( - <div className={CS.overflowHidden}> - <FormInput - name="name" - title={t`Name`} - placeholder={nameInputPlaceholder} - /> - <FormTextArea - name="description" - title={t`Description`} - placeholder={t`It's optional but oh, so helpful`} - /> - <FormCollectionPicker - name="collection_id" - title={t`Which collection should this go in?`} - zIndex={DEFAULT_MODAL_Z_INDEX + 1} - /> - </div> - )} - </TransitionGroup> - <FormFooter> - <FormErrorMessage inline /> - <Button - type="button" - onClick={onClose} - >{t`Cancel`}</Button> - <FormSubmitButton - title={t`Save`} - data-testid="save-question-button" - primary + <FormTextArea + name="description" + title={t`Description`} + placeholder={t`It's optional but oh, so helpful`} /> - </FormFooter> - </Form> - </Modal.Body> - </Modal.Content> - )} - </FormProvider> + <FormCollectionPicker + name="collection_id" + title={t`Which collection should this go in?`} + zIndex={DEFAULT_MODAL_Z_INDEX + 1} + /> + </div> + )} + </TransitionGroup> + <FormFooter> + <FormErrorMessage inline /> + <Button type="button" onClick={onClose}>{t`Cancel`}</Button> + <FormSubmitButton + title={t`Save`} + data-testid="save-question-button" + primary + /> + </FormFooter> + </Form> + </Modal.Body> + </Modal.Content> )} - </CreateCollectionOnTheGo> + </FormProvider> </Modal.Root> ); }; diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.tsx index 5addbb5ae8c..dd6ee1c52e6 100644 --- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.tsx +++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.tsx @@ -1,12 +1,10 @@ -import { useCallback, useEffect } from "react"; +import { useCallback, useEffect, useState } from "react"; import { t } from "ttag"; import { isPublicCollection } from "metabase/collections/utils"; +import { DashboardPickerModal } from "metabase/common/components/DashboardPicker"; +import { QuestionPickerModal } from "metabase/common/components/QuestionPicker"; import { useDashboardQuery } from "metabase/common/hooks"; -import ModalContent from "metabase/components/ModalContent"; -import ModalWithTrigger from "metabase/components/ModalWithTrigger"; -import DashboardPicker from "metabase/containers/DashboardPicker"; -import QuestionPicker from "metabase/containers/QuestionPicker"; import CS from "metabase/css/core/index.css"; import { ClickMappingsConnected, @@ -40,14 +38,14 @@ import { const LINK_TARGETS = { question: { Entity: Questions, - PickerComponent: QuestionPicker, + PickerComponent: QuestionPickerModal, pickerIcon: "bar" as const, getModalTitle: () => t`Pick a question to link to`, getPickerButtonLabel: () => t`Pick a question…`, }, dashboard: { Entity: Dashboards, - PickerComponent: DashboardPicker, + PickerComponent: DashboardPickerModal, pickerIcon: "dashboard" as const, getModalTitle: () => t`Pick a dashboard to link to`, getPickerButtonLabel: () => t`Pick a dashboard…`, @@ -59,9 +57,11 @@ const NO_DASHBOARD_TABS: DashboardTab[] = []; function PickerControl({ clickBehavior, onCancel, + onClick, }: { clickBehavior: EntityCustomDestinationClickBehavior; onCancel: () => void; + onClick?: () => void; }) { const { Entity, pickerIcon, getPickerButtonLabel } = LINK_TARGETS[clickBehavior.linkType]; @@ -76,7 +76,7 @@ function PickerControl({ return ( <SidebarItem.Selectable isSelected padded={false}> - <LinkTargetEntityPickerContent> + <LinkTargetEntityPickerContent onClick={onClick}> <SelectedEntityPickerIcon name={pickerIcon} /> <SelectedEntityPickerContent> {renderLabel()} @@ -138,7 +138,9 @@ export function LinkedEntityPicker({ const { linkType, targetId } = clickBehavior; const isDashboard = linkType === "dashboard"; const hasSelectedTarget = clickBehavior.targetId != null; - const { PickerComponent, getModalTitle } = LINK_TARGETS[linkType]; + const { getModalTitle, PickerComponent } = LINK_TARGETS[linkType]; + + const [isPickerOpen, setIsPickerOpen] = useState(!hasSelectedTarget); const handleSelectLinkTargetEntityId = useCallback( (targetId: CardId | DashboardId) => { @@ -246,37 +248,34 @@ export function LinkedEntityPicker({ ? "exclude" : undefined; + const initialPickerValue = + typeof targetId === "number" + ? { id: targetId, model: linkType === "dashboard" ? "dashboard" : "card" } + : { id: "root", model: "collection" }; + return ( - <div> - <div className={CS.pb1}> - <ModalWithTrigger - triggerElement={ - <PickerControl - clickBehavior={clickBehavior} - onCancel={handleResetLinkTargetType} - /> - } - isInitiallyOpen={!hasSelectedTarget} - > - {({ onClose }: { onClose: () => void }) => ( - <ModalContent - title={getModalTitle()} - onClose={hasSelectedTarget ? onClose : undefined} - > - {/* TODO: drop maxHeight when PickerComponent is migrated to TS */} - <PickerComponent - filterPersonalCollections={filterPersonalCollections} - value={clickBehavior.targetId} - onChange={(targetId: CardId | DashboardId) => { - handleSelectLinkTargetEntityId(targetId); - onClose(); - }} - maxHeight={undefined} - /> - </ModalContent> - )} - </ModalWithTrigger> - </div> + <> + <PickerControl + clickBehavior={clickBehavior} + onClick={() => setIsPickerOpen(true)} + onCancel={handleResetLinkTargetType} + /> + {isPickerOpen && ( + <PickerComponent + title={getModalTitle()} + value={initialPickerValue as any} // typescript isn't smart enough to know which picker we're using + onChange={newTarget => { + handleSelectLinkTargetEntityId(newTarget.id); + setIsPickerOpen(false); + }} + onClose={() => setIsPickerOpen(false)} + options={{ + showPersonalCollections: filterPersonalCollections !== "exclude", + showRootCollection: true, + hasConfirmButtons: false, + }} + /> + )} {isDashboard && dashboardTabs.length > 1 && ( <Select @@ -304,6 +303,6 @@ export function LinkedEntityPicker({ updateSettings={updateSettings} /> )} - </div> + </> ); } diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.unit.spec.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.unit.spec.tsx index 4ab7a8dc429..3f01d5f1dd4 100644 --- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.unit.spec.tsx +++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/LinkOptions/LinkedEntityPicker/LinkedEntityPicker.unit.spec.tsx @@ -2,11 +2,17 @@ import userEvent from "@testing-library/user-event"; import fetchMock from "fetch-mock"; import { + setupCollectionByIdEndpoint, setupCollectionItemsEndpoint, setupCollectionsEndpoints, setupSearchEndpoints, } from "__support__/server-mocks"; -import { renderWithProviders, screen } from "__support__/ui"; +import { + mockGetBoundingClientRect, + mockScrollBy, + renderWithProviders, + screen, +} from "__support__/ui"; import { getNextId } from "__support__/utils"; import { ROOT_COLLECTION as ROOT } from "metabase/entities/collections"; import { checkNotNull } from "metabase/lib/types"; @@ -24,12 +30,19 @@ import { createMockDashboard, createMockDashboardCard, createMockSearchResult, + createMockUser, } from "metabase-types/api/mocks"; import type { StoreDashboard } from "metabase-types/store"; import { createMockDashboardState } from "metabase-types/store/mocks"; import { LinkedEntityPicker } from "./LinkedEntityPicker"; +const CURRENT_USER = createMockUser({ + id: getNextId(), + personal_collection_id: getNextId(), + is_superuser: true, +}); + const ROOT_COLLECTION = createMockCollection({ ...ROOT, can_write: true, @@ -43,8 +56,15 @@ const PUBLIC_COLLECTION = createMockCollection({ location: "/", }); +const collectionInRootCollectionItem = createMockCollectionItem({ + id: PUBLIC_COLLECTION.id as number, + name: PUBLIC_COLLECTION.name, + model: "collection", + collection_id: PUBLIC_COLLECTION.id as number, +}); + const PERSONAL_COLLECTION = createMockCollection({ - id: getNextId(), + id: CURRENT_USER.personal_collection_id, name: "Personal collection", can_write: true, is_personal: true, @@ -66,12 +86,26 @@ function setup({ searchResults = [], collectionItems = [], }: SetupOpts) { + mockScrollBy(); + mockGetBoundingClientRect(); setupCollectionsEndpoints({ collections: COLLECTIONS }); - setupSearchEndpoints(searchResults); + + setupCollectionByIdEndpoint({ collections: COLLECTIONS }), + setupSearchEndpoints(searchResults); setupCollectionItemsEndpoint({ collection: ROOT_COLLECTION, collectionItems, }); + setupCollectionItemsEndpoint({ + collection: PERSONAL_COLLECTION, + collectionItems: [], + }); + setupCollectionItemsEndpoint({ + collection: PUBLIC_COLLECTION, + collectionItems: [], + }); + + fetchMock.get("path:/api/user/recipients", { data: [] }); renderWithProviders( <LinkedEntityPicker @@ -81,6 +115,7 @@ function setup({ />, { storeInitialState: { + currentUser: CURRENT_USER, dashboard: createMockDashboardState({ dashboardId: dashboard.id, dashboards: { @@ -122,17 +157,20 @@ describe("LinkedEntityPicker", () => { setup({ clickBehavior, dashboard: dashboardInPublicCollection, - collectionItems: [dashboardCollectionItem], + collectionItems: [ + collectionInRootCollectionItem, + dashboardCollectionItem, + ], }); expect( - screen.getByText("Pick a dashboard to link to"), + await screen.findByText("Pick a dashboard to link to"), ).toBeInTheDocument(); expect( await screen.findByText(PUBLIC_COLLECTION.name), ).toBeInTheDocument(); expect( - screen.queryByText(PERSONAL_COLLECTION.name), + screen.queryByText(/personal collection/i), ).not.toBeInTheDocument(); expect( await screen.findByText(dashboardCollectionItem.name), @@ -146,11 +184,11 @@ describe("LinkedEntityPicker", () => { dashboard: dashboardInPublicCollection, searchResults: [dashboardSearchResult], }); - await userEvent.click(screen.getByRole("button", { name: "Search" })); + expect(screen.getByText(/Pick a dashboard/i)).toBeInTheDocument(); const typedText = "dashboard"; await userEvent.type( - screen.getByPlaceholderText("Search"), - `${typedText}{enter}`, + await screen.findByPlaceholderText(/search/i), + typedText, ); expect( @@ -179,11 +217,14 @@ describe("LinkedEntityPicker", () => { setup({ clickBehavior, dashboard: dashboardInPersonalCollection, - collectionItems: [dashboardCollectionItem], + collectionItems: [ + collectionInRootCollectionItem, + dashboardCollectionItem, + ], }); expect( - screen.getByText("Pick a dashboard to link to"), + await screen.findByText(/Pick a dashboard to link/), ).toBeInTheDocument(); expect( await screen.findByText(PUBLIC_COLLECTION.name), @@ -195,17 +236,19 @@ describe("LinkedEntityPicker", () => { }); describe("search dashboards", () => { - it("should search dashboards only in public collections", async () => { + it("should search all dashboards", async () => { setup({ clickBehavior, dashboard: dashboardInPersonalCollection, searchResults: [dashboardSearchResult], }); - await userEvent.click(screen.getByRole("button", { name: "Search" })); + expect( + await screen.findByText(/Pick a dashboard/), + ).toBeInTheDocument(); const typedText = "dashboard"; await userEvent.type( - screen.getByPlaceholderText("Search"), - `${typedText}{enter}`, + await screen.findByPlaceholderText(/search/i), + typedText, ); expect( @@ -248,11 +291,14 @@ describe("LinkedEntityPicker", () => { setup({ clickBehavior, dashboard: dashboardInPublicCollection, - collectionItems: [questionCollectionItem], + collectionItems: [ + collectionInRootCollectionItem, + questionCollectionItem, + ], }); expect( - screen.getByText("Pick a question to link to"), + await screen.findByText("Pick a question to link to"), ).toBeInTheDocument(); expect( await screen.findByText(PUBLIC_COLLECTION.name), @@ -266,17 +312,19 @@ describe("LinkedEntityPicker", () => { }); describe("questions", () => { - it("should search questions in all collections", async () => { + it("should search questions only in public collections", async () => { setup({ clickBehavior, dashboard: dashboardInPublicCollection, searchResults: [questionSearchResult], }); - await userEvent.click(screen.getByRole("button", { name: "Search" })); + expect( + await screen.findByText(/Pick a question/), + ).toBeInTheDocument(); const typedText = "question"; await userEvent.type( - screen.getByPlaceholderText("Search"), - `${typedText}{enter}`, + await screen.findByPlaceholderText(/search/i), + typedText, ); expect( @@ -310,11 +358,14 @@ describe("LinkedEntityPicker", () => { setup({ clickBehavior, dashboard: dashboardInPersonalCollection, - collectionItems: [questionCollectionItem], + collectionItems: [ + collectionInRootCollectionItem, + questionCollectionItem, + ], }); expect( - screen.getByText("Pick a question to link to"), + await screen.findByText("Pick a question to link to"), ).toBeInTheDocument(); expect( await screen.findByText(PUBLIC_COLLECTION.name), @@ -332,11 +383,10 @@ describe("LinkedEntityPicker", () => { dashboard: dashboardInPersonalCollection, searchResults: [questionSearchResult], }); - await userEvent.click(screen.getByRole("button", { name: "Search" })); const typedText = "question"; await userEvent.type( - screen.getByPlaceholderText("Search"), - `${typedText}{enter}`, + await screen.findByPlaceholderText(/search/i), + typedText, ); expect( diff --git a/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx b/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx index 3f9a02a1c67..ef2c256f627 100644 --- a/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx +++ b/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx @@ -5,7 +5,7 @@ import _ from "underscore"; import * as Yup from "yup"; import FormCollectionPicker from "metabase/collections/containers/FormCollectionPicker/FormCollectionPicker"; -import type { FilterItemsInPersonalCollection } from "metabase/containers/ItemPicker"; +import type { FilterItemsInPersonalCollection } from "metabase/common/components/EntityPicker"; import Button from "metabase/core/components/Button"; import FormFooter from "metabase/core/components/FormFooter"; import Dashboards from "metabase/entities/dashboards"; diff --git a/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx b/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx index b48174dd8b6..44d432fe5e3 100644 --- a/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx +++ b/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx @@ -6,7 +6,7 @@ import _ from "underscore"; import * as Yup from "yup"; import FormCollectionPicker from "metabase/collections/containers/FormCollectionPicker/FormCollectionPicker"; -import type { FilterItemsInPersonalCollection } from "metabase/containers/ItemPicker"; +import type { FilterItemsInPersonalCollection } from "metabase/common/components/EntityPicker"; import Button from "metabase/core/components/Button"; import FormErrorMessage from "metabase/core/components/FormErrorMessage"; import FormFooter from "metabase/core/components/FormFooter"; diff --git a/frontend/src/metabase/dashboard/containers/CreateDashboardModal.tsx b/frontend/src/metabase/dashboard/containers/CreateDashboardModal.tsx index 471610219dc..faf96939b20 100644 --- a/frontend/src/metabase/dashboard/containers/CreateDashboardModal.tsx +++ b/frontend/src/metabase/dashboard/containers/CreateDashboardModal.tsx @@ -5,7 +5,6 @@ import { push } from "react-router-redux"; import { t } from "ttag"; import ModalContent from "metabase/components/ModalContent"; -import { CreateCollectionOnTheGo } from "metabase/containers/CreateCollectionOnTheGo"; import * as Urls from "metabase/lib/urls"; import type { Dashboard } from "metabase-types/api"; import type { State } from "metabase-types/store"; @@ -47,22 +46,17 @@ function CreateDashboardModal({ ); return ( - <CreateCollectionOnTheGo> - {({ resumedValues }) => ( - <ModalContent - title={t`New dashboard`} - onClose={onClose} - data-testid="new-dashboard-modal" - > - <CreateDashboardFormConnected - {...props} - onCreate={handleCreate} - onCancel={onClose} - initialValues={resumedValues} - /> - </ModalContent> - )} - </CreateCollectionOnTheGo> + <ModalContent + title={t`New dashboard`} + onClose={onClose} + data-testid="new-dashboard-modal" + > + <CreateDashboardFormConnected + {...props} + onCreate={handleCreate} + onCancel={onClose} + /> + </ModalContent> ); } diff --git a/frontend/src/metabase/entities/containers/EntityCopyModal.tsx b/frontend/src/metabase/entities/containers/EntityCopyModal.tsx index 595f6ff4224..6bde838f143 100644 --- a/frontend/src/metabase/entities/containers/EntityCopyModal.tsx +++ b/frontend/src/metabase/entities/containers/EntityCopyModal.tsx @@ -7,7 +7,6 @@ import { } from "metabase/collections/utils"; import { useCollectionListQuery } from "metabase/common/hooks"; import ModalContent from "metabase/components/ModalContent"; -import { CreateCollectionOnTheGo } from "metabase/containers/CreateCollectionOnTheGo"; import { CopyDashboardFormConnected } from "metabase/dashboard/containers/CopyDashboardForm"; import { CopyQuestionForm } from "metabase/questions/components/CopyQuestionForm"; import { Flex, Loader } from "metabase/ui"; @@ -78,22 +77,18 @@ const EntityCopyModal = ({ }; return ( - <CreateCollectionOnTheGo> - {({ resumedValues }) => ( - <ModalContent - title={title || t`Duplicate "${resolvedObject.name}"`} - onClose={onClose} - > - {!collections?.length ? ( - <Flex justify="center" p="lg"> - <Loader /> - </Flex> - ) : ( - renderForm({ ...props, resumedValues, initialValues }) - )} - </ModalContent> + <ModalContent + title={title || t`Duplicate "${resolvedObject.name}"`} + onClose={onClose} + > + {!collections?.length ? ( + <Flex justify="center" p="lg"> + <Loader /> + </Flex> + ) : ( + renderForm({ ...props, initialValues }) )} - </CreateCollectionOnTheGo> + </ModalContent> ); }; diff --git a/frontend/src/metabase/models/containers/FormModelPicker/FormModelPicker.styled.tsx b/frontend/src/metabase/models/containers/FormModelPicker/FormModelPicker.styled.tsx deleted file mode 100644 index ad80cf0d02f..00000000000 --- a/frontend/src/metabase/models/containers/FormModelPicker/FormModelPicker.styled.tsx +++ /dev/null @@ -1,11 +0,0 @@ -import styled from "@emotion/styled"; - -import ItemPicker from "metabase/containers/ItemPicker"; - -export const MIN_POPOVER_WIDTH = 300; - -export const PopoverItemPicker = styled(ItemPicker)<{ width: number }>` - width: ${({ width = MIN_POPOVER_WIDTH }) => width}px; - padding: 1rem; - overflow: auto; -`; diff --git a/frontend/src/metabase/timelines/collections/routes.tsx b/frontend/src/metabase/timelines/collections/routes.tsx index ee24d5f29a9..ca82792b5e9 100644 --- a/frontend/src/metabase/timelines/collections/routes.tsx +++ b/frontend/src/metabase/timelines/collections/routes.tsx @@ -59,6 +59,7 @@ const getRoutes = () => { path: "timelines/:timelineId/move", modal: MoveTimelineModal, modalProps: { enableTransition: false }, + noWrap: true, }} /> <ModalRoute diff --git a/frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.styled.tsx b/frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.styled.tsx deleted file mode 100644 index 108836a3323..00000000000 --- a/frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.styled.tsx +++ /dev/null @@ -1,17 +0,0 @@ -import styled from "@emotion/styled"; - -export const ModalRoot = styled.div` - display: flex; - flex-direction: column; - min-height: 573px; - max-height: 90vh; -`; - -export const ModalBody = styled.div` - display: flex; - flex-direction: column; - flex: 1 1 auto; - margin: 1rem 0; - padding: 1rem 2rem; - overflow-y: auto; -`; diff --git a/frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.tsx b/frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.tsx index fd328234678..5f43b9b1e34 100644 --- a/frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.tsx +++ b/frontend/src/metabase/timelines/common/components/MoveTimelineModal/MoveTimelineModal.tsx @@ -1,59 +1,47 @@ -import { useCallback, useState } from "react"; +import { useCallback } from "react"; import { t } from "ttag"; -import CollectionPicker from "metabase/containers/CollectionPicker"; -import Button from "metabase/core/components/Button/Button"; +import { CollectionPickerModal } from "metabase/common/components/CollectionPicker"; import { getTimelineName } from "metabase/lib/timelines"; -import type { Timeline } from "metabase-types/api"; - -import ModalFooter from "../ModalFooter"; -import ModalHeader from "../ModalHeader"; - -import { ModalBody, ModalRoot } from "./MoveTimelineModal.styled"; +import type { CollectionId, Timeline } from "metabase-types/api"; export interface MoveTimelineModalProps { timeline: Timeline; - onSubmit: (timeline: Timeline, collectionId: number | null) => void; + onSubmit: (timeline: Timeline, collectionId: CollectionId) => void; onSubmitSuccess?: () => void; onCancel?: () => void; - onClose?: () => void; + onClose: () => void; } const MoveTimelineModal = ({ timeline, onSubmit, onSubmitSuccess, - onCancel, onClose, }: MoveTimelineModalProps): JSX.Element => { - const [collectionId, setCollectionId] = useState(timeline.collection_id); - const isEnabled = timeline.collection_id !== collectionId; - - const handleSubmit = useCallback(async () => { - await onSubmit(timeline, collectionId); - onSubmitSuccess?.(); - }, [timeline, collectionId, onSubmit, onSubmitSuccess]); + const handleSubmit = useCallback( + async (collectionId: CollectionId) => { + await onSubmit(timeline, collectionId); + onSubmitSuccess?.(); + onClose?.(); + }, + [timeline, onSubmit, onSubmitSuccess, onClose], + ); return ( - <ModalRoot> - <ModalHeader - title={t`Move ${getTimelineName(timeline)}`} - onClose={onClose} - /> - <ModalBody> - <CollectionPicker - value={collectionId} - showScroll={false} - onChange={setCollectionId} - /> - </ModalBody> - <ModalFooter> - <Button onClick={onCancel}>{t`Cancel`}</Button> - <Button primary disabled={!isEnabled} onClick={handleSubmit}> - {t`Move`} - </Button> - </ModalFooter> - </ModalRoot> + <CollectionPickerModal + value={{ id: timeline.collection_id ?? "root", model: "collection" }} + title={t`Move ${getTimelineName(timeline)}`} + onClose={onClose} + onChange={async newCollection => { + await handleSubmit(newCollection.id); + }} + options={{ + confirmButtonText: t`Move`, + showPersonalCollections: true, + showRootCollection: true, + }} + /> ); }; -- GitLab