diff --git a/e2e/test/scenarios/admin/admin-reproductions.cy.spec.js b/e2e/test/scenarios/admin/admin-reproductions.cy.spec.js index d5b2b427a66256e753fa24ddc48636f3c921cdfe..aca8916407ed0c66c3665aa50798532ad4cc5207 100644 --- a/e2e/test/scenarios/admin/admin-reproductions.cy.spec.js +++ b/e2e/test/scenarios/admin/admin-reproductions.cy.spec.js @@ -5,6 +5,7 @@ import { getNotebookStep, popover, queryWritableDB, + relativeDatePicker, resetTestTable, restore, resyncDatabase, @@ -168,3 +169,69 @@ describe("(metabase#45042)", () => { cy.findByRole("list", { name: "Navigation links" }).should("not.exist"); }); }); + +describe("(metabase#46714)", () => { + beforeEach(() => { + restore(); + cy.signInAsAdmin(); + + cy.visit("/admin/datamodel/segment/create"); + + cy.findByTestId("gui-builder").findByText("Select a table").click(); + + popover().within(() => { + cy.findByText("Orders").click(); + }); + + cy.findByTestId("gui-builder") + .findByText("Add filters to narrow your answer") + .click(); + }); + + it("should allow users to apply relative date options in the segment date picker", () => { + popover().within(() => { + cy.findByText("Created At").click(); + cy.findByText("Relative dates...").click(); + cy.findByRole("button", { name: "Previous" }).click(); + cy.findByLabelText("Options").click(); + }); + + cy.findByTestId("relative-date-picker-options").within(() => { + cy.findByText("Starting from...").click(); + }); + + relativeDatePicker.setValue({ value: 68, unit: "day" }); + + relativeDatePicker.setStartingFrom({ + value: 70, + unit: "day", + }); + + popover().findByText("Add filter").click(); + + cy.findByTestId("filter-widget-target").should( + "have.text", + "Created At Previous 68 Days, starting 70 days ago", + ); + }); + + it("should not hide operator select menu behind the main filter popover", () => { + popover().within(() => { + cy.findByText("Total").click(); + }); + + cy.findByTestId("operator-select").should("have.value", "Equal to").click(); + cy.findByTestId("select-dropdown") + .should("exist") + .findByText("Less than") + .click(); + cy.findByTestId("operator-select").should("have.value", "Less than"); + cy.findByTestId("field-values-widget").clear().type("1000"); + popover().findByText("Add filter").click(); + + cy.findByTestId("filter-widget-target").should( + "have.text", + "Total is less than 1000", + ); + }); +}); diff --git a/e2e/test/scenarios/admin/datamodel/datamodel.cy.spec.js b/e2e/test/scenarios/admin/datamodel/datamodel.cy.spec.js index 5fef93efa945f066ae3aee7af35fe088bb91fd53..b7fd5c78a06f6574bea11b92bb73663e9d4f5389 100644 --- a/e2e/test/scenarios/admin/datamodel/datamodel.cy.spec.js +++ b/e2e/test/scenarios/admin/datamodel/datamodel.cy.spec.js @@ -822,35 +822,34 @@ describe("scenarios > admin > datamodel > segments", () => { it("should update that segment", () => { cy.visit("/admin"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Table Metadata").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Segments").click(); + cy.findByTestId("admin-navbar-items").contains("Table Metadata").click(); + cy.get("label").contains("Segments").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains(SEGMENT_NAME) + cy.findByTestId("segment-list-app") + .contains(SEGMENT_NAME) .parent() .parent() .parent() .find(".Icon-ellipsis") .click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Edit Segment").click(); + popover().contains("Edit Segment").click(); // update the filter from "< 100" to "> 10" cy.url().should("match", /segment\/1$/); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Edit Your Segment"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains(/Total\s+is less than/).click(); - popover().contains("Less than").click(); + cy.get("label").contains("Edit Your Segment"); + cy.findByTestId("filter-widget-target") + .contains(/Total\s+is less than/) + .click(); + popover().findByTestId("operator-select").click(); popover().contains("Greater than").click(); - popover().find("input").type("{SelectAll}10"); + popover() + .findByTestId("field-values-widget") + .find("input") + .type("{SelectAll}10"); popover().contains("Update filter").click(); // confirm that the preview updated - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("18758 rows"); + cy.findByTestId("gui-builder").contains("18758 rows"); // update name and description, set a revision note, and save the update cy.get('[name="name"]').clear().type("Orders > 10"); @@ -858,24 +857,22 @@ describe("scenarios > admin > datamodel > segments", () => { .clear() .type("All orders with a total over $10."); cy.get('[name="revision_message"]').type("time for a change"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Save changes").click(); + cy.findByTestId("field-set-content").findByText("Save changes").click(); // get redirected to previous page and see the new segment name cy.url().should("match", /datamodel\/segments$/); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Orders > 10"); + + cy.findByTestId("segment-list-app").findByText("Orders > 10"); // clean up - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Orders > 10") + cy.findByTestId("segment-list-app") + .contains("Orders > 10") .parent() .parent() .parent() .find(".Icon-ellipsis") .click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.contains("Retire Segment").click(); + popover().findByText("Retire Segment").click(); modal().find("textarea").type("delete it"); modal().contains("button", "Retire").click(); }); diff --git a/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx b/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx index 631ddc5a455f3e94ed8448df62e43b27e93dfecf..66b3fb2b32a737d399e75264d756689ba81325d4 100644 --- a/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx +++ b/frontend/src/metabase/admin/datamodel/components/FilterPopover/FilterPopover.unit.spec.tsx @@ -76,7 +76,7 @@ describe("FilterPopover", () => { describe("filter operator selection", () => { it("should have an operator selector", () => { setup({ filter: NUMERIC_FILTER }); - expect(screen.getByText("Equal to")).toBeInTheDocument(); + expect(screen.getByTestId("operator-select")).toHaveValue("Equal to"); expect(screen.getByText("1234")).toBeInTheDocument(); }); }); diff --git a/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.styled.tsx b/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.styled.tsx index cb8166193fffc41936c0480534df53cd300f0992..6556c98c27283ff9f4e939e751814cc90e8a248d 100644 --- a/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.styled.tsx +++ b/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.styled.tsx @@ -10,6 +10,7 @@ export const FilterWidgetRoot = styled.div<FilterWidgetRootProps>` flex-shrink: 0; border: 2px solid transparent; border-radius: 0.5rem; + cursor: pointer; ${({ isSelected }) => isSelected && diff --git a/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.tsx b/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.tsx index c6312dce6fab1d9ec494d1e27d887320c2b12e07..d063d90258e4774d2046c52735955ac699219d26 100644 --- a/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.tsx +++ b/frontend/src/metabase/admin/datamodel/components/FilterWidget/FilterWidget.tsx @@ -1,13 +1,12 @@ +import { useDisclosure } from "@mantine/hooks"; import cx from "classnames"; -import { Component } from "react"; -import * as React from "react"; -import { Filter as FilterComponent } from "metabase/admin/datamodel/components/Filter"; -import Popover from "metabase/components/Popover"; +import { Filter } from "metabase/admin/datamodel/components/Filter"; import CS from "metabase/css/core/index.css"; import QueryBuilderS from "metabase/css/query_builder.module.css"; +import { Popover } from "metabase/ui"; import type StructuredQuery from "metabase-lib/v1/queries/StructuredQuery"; -import type Filter from "metabase-lib/v1/queries/structured/Filter"; +import type FilterType from "metabase-lib/v1/queries/structured/Filter"; import { FilterPopover } from "../FilterPopover"; @@ -65,93 +64,59 @@ export const filterWidgetFilterRenderer = ({ ); type Props = { - filter: Filter; + filter: FilterType; query: StructuredQuery; updateFilter: (index: number, filter: any[]) => void; index: number; removeFilter: (index: number) => void; -}; - -type State = { - isOpen: boolean; + maxDisplayValues?: number; }; /** * @deprecated use MLv2 */ -export class FilterWidget extends Component<Props, State> { - rootRef: React.RefObject<HTMLDivElement>; - - constructor(props: Props) { - super(props); - - this.state = { - isOpen: this.props.filter[0] == null, - }; - - this.rootRef = React.createRef(); - } - - static defaultProps = { - maxDisplayValues: 1, - }; - - open = () => { - this.setState({ isOpen: true }); - }; - - close = () => { - this.setState({ isOpen: false }); - }; - - renderFilter() { - const { query } = this.props; - return ( - <FilterComponent - metadata={query && query.metadata && query.metadata()} - {...this.props} - > - {filterWidgetFilterRenderer} - </FilterComponent> - ); - } - - renderPopover() { - if (this.state.isOpen) { - const { query, filter } = this.props; - return ( - <Popover - id="FilterPopover" - className="FilterPopover" - target={this.rootRef.current} - isInitiallyOpen={this.props.filter[1] === null} - onClose={this.close} - horizontalAttachments={["left", "center"]} - autoWidth +export const FilterWidget = ({ + filter, + index, + query, + removeFilter, + updateFilter, + maxDisplayValues = 1, +}: Props) => { + const [opened, { close, open }] = useDisclosure(filter[0] == null); + + return ( + <Popover opened={opened} onClose={close}> + <Popover.Target> + <FilterWidgetRoot + data-testid="filter-widget-target" + isSelected={opened} > - <FilterPopover - query={query} - filter={filter} - onChangeFilter={filter => { - this.props.updateFilter?.(this.props.index, filter); - this.close(); - }} - onClose={this.close} - isNew={false} - /> - </Popover> - ); - } - } - - render() { - return ( - <FilterWidgetRoot isSelected={this.state.isOpen} ref={this.rootRef}> - <div className={cx(CS.flex, CS.justifyCenter)} onClick={this.open}> - {this.renderFilter()} - </div> - {this.renderPopover()} - </FilterWidgetRoot> - ); - } -} + <div className={cx(CS.flex, CS.justifyCenter)} onClick={open}> + <Filter + metadata={query?.metadata?.()} + filter={filter} + index={index} + removeFilter={removeFilter} + maxDisplayValues={maxDisplayValues} + > + {filterWidgetFilterRenderer} + </Filter> + </div> + </FilterWidgetRoot> + </Popover.Target> + <Popover.Dropdown> + <FilterPopover + query={query} + filter={filter} + onChangeFilter={filter => { + updateFilter?.(index, filter); + close(); + }} + onClose={close} + isNew={false} + /> + </Popover.Dropdown> + </Popover> + ); +}; diff --git a/frontend/src/metabase/admin/datamodel/components/GuiQueryEditor/GuiQueryEditor.jsx b/frontend/src/metabase/admin/datamodel/components/GuiQueryEditor/GuiQueryEditor.jsx index 36bbc1cf7371a7aafc43b30942383524fc8703ef..531e7aaa386a5cfd34434864f8b508f653198508 100644 --- a/frontend/src/metabase/admin/datamodel/components/GuiQueryEditor/GuiQueryEditor.jsx +++ b/frontend/src/metabase/admin/datamodel/components/GuiQueryEditor/GuiQueryEditor.jsx @@ -7,11 +7,10 @@ import ReactDOM from "react-dom"; import { t } from "ttag"; import IconBorder from "metabase/components/IconBorder"; -import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; import CS from "metabase/css/core/index.css"; import QueryBuilderS from "metabase/css/query_builder.module.css"; import { DatabaseSchemaAndTableDataSelector } from "metabase/query_builder/components/DataSelector"; -import { Icon } from "metabase/ui"; +import { Icon, Popover, Box } from "metabase/ui"; import * as Lib from "metabase-lib"; import { FilterPopover } from "../FilterPopover"; @@ -26,12 +25,12 @@ export class GuiQueryEditor extends Component { constructor(props) { super(props); - this.filterPopover = createRef(); this.guiBuilder = createRef(); } state = { expanded: true, + isFilterPopoverOpen: false, }; static propTypes = { @@ -139,23 +138,34 @@ export class GuiQueryEditor extends Component { > <div className={QueryBuilderS.QueryFilters}>{filterList}</div> <div className={CS.mx2}> - <PopoverWithTrigger - id="FilterPopover" - ref={this.filterPopover} - triggerElement={addFilterButton} - triggerClasses={cx(CS.flex, CS.alignCenter)} - horizontalAttachments={["left", "center"]} - autoWidth + <Popover + opened={this.state.isFilterPopoverOpen} + onClose={() => { + this.setState({ isFilterPopoverOpen: false }); + }} > - <FilterPopover - isNew - query={legacyQuery} - onChangeFilter={filter => - setDatasetQuery(legacyQuery.filter(filter)) - } - onClose={() => this.filterPopover.current.close()} - /> - </PopoverWithTrigger> + <Popover.Target> + <Box + onClick={() => + this.setState({ + isFilterPopoverOpen: !this.state.isFilterPopoverOpen, + }) + } + > + {addFilterButton} + </Box> + </Popover.Target> + <Popover.Dropdown> + <FilterPopover + isNew + query={legacyQuery} + onChangeFilter={filter => + setDatasetQuery(legacyQuery.filter(filter)) + } + onClose={() => this.setState({ isFilterPopoverOpen: false })} + /> + </Popover.Dropdown> + </Popover> </div> </div> ); diff --git a/frontend/src/metabase/admin/datamodel/components/filters/OperatorSelector.jsx b/frontend/src/metabase/admin/datamodel/components/filters/OperatorSelector.jsx deleted file mode 100644 index 63f3c2401a91f7facd078e9f259bb364afa46b1e..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/admin/datamodel/components/filters/OperatorSelector.jsx +++ /dev/null @@ -1,34 +0,0 @@ -/* eslint-disable react/prop-types */ -import cx from "classnames"; -import PropTypes from "prop-types"; -import { Component } from "react"; - -import Select, { Option } from "metabase/core/components/Select"; -import CS from "metabase/css/core/index.css"; - -export default class OperatorSelector extends Component { - static propTypes = { - operator: PropTypes.string, - operators: PropTypes.array.isRequired, - onOperatorChange: PropTypes.func.isRequired, - }; - - render() { - const { operator, operators, onOperatorChange, className } = this.props; - - return ( - <Select - value={operator} - onChange={e => onOperatorChange(e.target.value)} - className={cx(CS.borderMedium, CS.textDefault, className)} - data-testid="operator-select" - > - {operators.map(o => ( - <Option key={o.name} value={o.name}> - {o.verboseName} - </Option> - ))} - </Select> - ); - } -} diff --git a/frontend/src/metabase/admin/datamodel/components/filters/OperatorSelector.tsx b/frontend/src/metabase/admin/datamodel/components/filters/OperatorSelector.tsx new file mode 100644 index 0000000000000000000000000000000000000000..849389ff69096aa348cdad6550c1678fcd92508d --- /dev/null +++ b/frontend/src/metabase/admin/datamodel/components/filters/OperatorSelector.tsx @@ -0,0 +1,29 @@ +import cx from "classnames"; + +import CS from "metabase/css/core/index.css"; +import { Select } from "metabase/ui"; +import type { FilterOperator } from "metabase-lib/v1/deprecated-types"; + +// Using deprecated types from metabase-lib v1 to avoid breaking changes. We should update this ASAP +const OperatorSelector = ({ + operator, + operators, + onOperatorChange, + className, +}: { + operator: FilterOperator["name"]; + operators: FilterOperator[] | null | undefined; + onOperatorChange: (operator: FilterOperator["name"]) => void; + className?: string; +}) => ( + <Select + data={(operators ?? []).map(o => ({ value: o.name, label: o.verboseName }))} + value={operator} + onChange={onOperatorChange} + className={cx(CS.borderMedium, CS.textDefault, className)} + data-testid="operator-select" + /> +); + +// eslint-disable-next-line import/no-default-export -- deprecated usage +export default OperatorSelector; diff --git a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DatePicker/RelativeDatePicker.tsx b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DatePicker/RelativeDatePicker.tsx index 2405e9818b9a715006f304d6fa352a61350a827f..34b37c452f310c9aae0395da04660ae873e807c6 100644 --- a/frontend/src/metabase/admin/datamodel/components/filters/pickers/DatePicker/RelativeDatePicker.tsx +++ b/frontend/src/metabase/admin/datamodel/components/filters/pickers/DatePicker/RelativeDatePicker.tsx @@ -127,7 +127,7 @@ const OptionsContent = ({ }; return ( - <OptionsContainer> + <OptionsContainer data-testid="relative-date-picker-options"> {supportsExpressions && ( <OptionButton icon="arrow_left_to_line" diff --git a/frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx b/frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx index 73a42efb00bb564ecd1c10d97374c1280949c690..8bf23d5b3f029024dc39449bc9171793fa809f99 100644 --- a/frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx +++ b/frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx @@ -18,7 +18,7 @@ class SegmentListAppInner extends Component { const { segments, tableSelector, setArchived } = this.props; return ( - <div className={cx(CS.px3, CS.pb2)}> + <div className={cx(CS.px3, CS.pb2)} data-testid="segment-list-app"> <div className={cx(CS.flex, CS.py2)}> {tableSelector} <Link to="/admin/datamodel/segment/create" className={CS.mlAuto}> diff --git a/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx b/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx index b4172db254b8b1c897af497b600dfadba094f942..0aa0af12e2b0609ad443f8974473e1a68d13f3ab 100644 --- a/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx +++ b/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx @@ -56,7 +56,7 @@ export const AdminNavbar = ({ <MobileNavbar adminPaths={adminPaths} currentPath={currentPath} /> <MobileHide> - <AdminNavbarItems> + <AdminNavbarItems data-testid="admin-navbar-items"> {adminPaths.map(({ name, key, path }) => ( <AdminNavItem name={name}