diff --git a/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js b/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js index a09db438a3a4ddf76be8333e769ceef6c5339a5b..f790b6f8325bfb91ce206b46982d536cd3e8e3cf 100644 --- a/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js @@ -404,6 +404,49 @@ describe("scenarios > dashboard > temporal unit parameters", () => { }); describe("parameter settings", () => { + it("should be able to set available temporal units", () => { + createDashboardWithCard().then(dashboard => visitDashboard(dashboard.id)); + + editDashboard(); + editParameter(parameterDetails.name); + dashboardParameterSidebar().findByText("All").click(); + popover().within(() => { + cy.findByLabelText("Select none").click(); + cy.findByLabelText("Month").click(); + cy.findByLabelText("Year").click(); + cy.findByLabelText("Minute").click(); + }); + saveDashboard(); + + filterWidget().click(); + popover().within(() => { + cy.findByText("Minute").should("not.exist"); + cy.findByText("Day").should("not.exist"); + cy.findByText("Month").should("be.visible"); + cy.findByText("Year").should("be.visible").click(); + }); + getDashboardCard().findByText("Created At: Year").should("be.visible"); + }); + + it("should clear the default value if it is no longer within the allowed unit list", () => { + createDashboardWithCard().then(dashboard => visitDashboard(dashboard.id)); + + cy.log("set the default value"); + editDashboard(); + editParameter(parameterDetails.name); + dashboardParameterSidebar().findByText("No default").click(); + popover().findByText("Year").click(); + + cy.log("exclude an unrelated temporal unit"); + dashboardParameterSidebar().findByText("All").click(); + popover().findByLabelText("Month").click(); + dashboardParameterSidebar().findByText("No default").should("not.exist"); + + cy.log("exclude the temporal unit used for the default value"); + popover().findByLabelText("Year").click(); + dashboardParameterSidebar().findByText("No default").should("be.visible"); + }); + it("should be able to set the default value and make it required", () => { createDashboardWithCard().then(dashboard => cy.wrap(dashboard.id).as("dashboardId"), diff --git a/frontend/src/metabase-lib/filter.ts b/frontend/src/metabase-lib/filter.ts index e08c392ec78201144a52b1634ddcc74bd286f2f0..c36820775a8c828e83713da76b7c085b1bfdeb01 100644 --- a/frontend/src/metabase-lib/filter.ts +++ b/frontend/src/metabase-lib/filter.ts @@ -1,7 +1,7 @@ import moment from "moment-timezone"; // eslint-disable-line no-restricted-imports -- deprecated usage import * as ML from "cljs/metabase.lib.js"; -import type { DatasetColumn } from "metabase-types/api"; +import type { DatasetColumn, TemporalUnit } from "metabase-types/api"; import { isBoolean, @@ -36,7 +36,6 @@ import type { BooleanFilterOperatorName, BooleanFilterParts, Bucket, - BucketName, ColumnMetadata, CoordinateFilterOperatorName, CoordinateFilterParts, @@ -565,11 +564,11 @@ function findTemporalBucket( query: Query, stageIndex: number, column: ColumnMetadata, - bucketName: BucketName, + temporalUnit: TemporalUnit, ): Bucket | undefined { return availableTemporalBuckets(query, stageIndex, column).find(bucket => { const bucketInfo = displayInfo(query, stageIndex, bucket); - return bucketInfo.shortName === bucketName; + return bucketInfo.shortName === temporalUnit; }); } @@ -854,9 +853,9 @@ function serializeExcludeDatePart( function deserializeExcludeDatePart( value: ExpressionArg | ExpressionParts, - bucketName: BucketName, + temporalUnit: TemporalUnit, ): number | null { - if (bucketName === "hour-of-day") { + if (temporalUnit === "hour-of-day") { return isNumberLiteral(value) ? value : null; } @@ -869,7 +868,7 @@ function deserializeExcludeDatePart( return null; } - switch (bucketName) { + switch (temporalUnit) { case "day-of-week": return date.isoWeekday(); case "month-of-year": diff --git a/frontend/src/metabase-lib/temporal_bucket.ts b/frontend/src/metabase-lib/temporal_bucket.ts index 48579f6284d2310aa22c121d03698b5280f4976c..6b8e78bceb5c3fa684525617bbabb9689fb6b499 100644 --- a/frontend/src/metabase-lib/temporal_bucket.ts +++ b/frontend/src/metabase-lib/temporal_bucket.ts @@ -1,13 +1,8 @@ import * as ML from "cljs/metabase.lib.js"; +import type { TemporalUnit } from "metabase-types/api"; import { displayInfo } from "./metadata"; -import type { - Bucket, - ColumnMetadata, - Clause, - Query, - BucketName, -} from "./types"; +import type { Bucket, ColumnMetadata, Clause, Query } from "./types"; export function temporalBucket(clause: Clause | ColumnMetadata): Bucket | null { return ML.temporal_bucket(clause); @@ -52,23 +47,23 @@ type IntervalAmount = number | "current" | "next" | "last"; export function describeTemporalInterval( n: IntervalAmount, - unit?: BucketName, + unit?: TemporalUnit, ): string { return ML.describe_temporal_interval(n, unit); } export function describeRelativeDatetime( n: IntervalAmount, - unit?: BucketName, + unit?: TemporalUnit, ): string { return ML.describe_relative_datetime(n, unit); } type RelativeDateRangeFormatOpts = { value: number | "current"; - unit: BucketName; + unit: TemporalUnit; offsetValue?: number; - offsetUnit?: BucketName; + offsetUnit?: TemporalUnit; includeCurrent?: boolean; }; @@ -84,6 +79,6 @@ export function formatRelativeDateRange({ }); } -export function availableTemporalUnits(): string[] { +export function availableTemporalUnits(): TemporalUnit[] { return ML.available_temporal_units(); } diff --git a/frontend/src/metabase-lib/types.ts b/frontend/src/metabase-lib/types.ts index 7d4b076f1566c14d8f69d9551d3a06be637fccf7..4cc7196ad21613e99fb09c41423fa775bbe206d4 100644 --- a/frontend/src/metabase-lib/types.ts +++ b/frontend/src/metabase-lib/types.ts @@ -7,6 +7,7 @@ import type { RowValue, TableId, SchemaId, + TemporalUnit, } from "metabase-types/api"; import type { @@ -115,21 +116,8 @@ export type ColumnGroup = unknown & { _opaque: typeof ColumnGroup }; declare const Bucket: unique symbol; export type Bucket = unknown & { _opaque: typeof Bucket }; -export type BucketName = - | "minute" - | "hour" - | "day" - | "week" - | "quarter" - | "month" - | "year" - | "day-of-week" - | "month-of-year" - | "quarter-of-year" - | "hour-of-day"; - export type BucketDisplayInfo = { - shortName: BucketName; + shortName: TemporalUnit; displayName: string; default?: boolean; selected?: boolean; diff --git a/frontend/src/metabase-types/api/dataset.ts b/frontend/src/metabase-types/api/dataset.ts index 66fb10e340857e0b3a35fd3fe18a3aa03c99520e..aa36f50f994a9744938b23e55ac632c8e7911eff 100644 --- a/frontend/src/metabase-types/api/dataset.ts +++ b/frontend/src/metabase-types/api/dataset.ts @@ -153,3 +153,16 @@ export interface TemplateTag { } export type TemplateTags = Record<TemplateTagName, TemplateTag>; + +export type TemporalUnit = + | "minute" + | "hour" + | "day" + | "week" + | "quarter" + | "month" + | "year" + | "day-of-week" + | "month-of-year" + | "quarter-of-year" + | "hour-of-day"; diff --git a/frontend/src/metabase-types/api/parameters.ts b/frontend/src/metabase-types/api/parameters.ts index 28ab02bc4982fc4974e78bc14a7335945cf3d591..43ec950ba9088791ba0079275bafff205b7f8900 100644 --- a/frontend/src/metabase-types/api/parameters.ts +++ b/frontend/src/metabase-types/api/parameters.ts @@ -1,5 +1,5 @@ import type { CardId } from "./card"; -import type { RowValue } from "./dataset"; +import type { RowValue, TemporalUnit } from "./dataset"; import type { ConcreteFieldReference, ExpressionReference } from "./query"; export type StringParameterType = @@ -51,6 +51,7 @@ export interface Parameter extends ParameterValuesConfig { isMultiSelect?: boolean; value?: any; target?: ParameterTarget; + temporal_units?: TemporalUnit[]; } export interface ParameterValuesConfig { diff --git a/frontend/src/metabase/dashboard/actions/parameters.ts b/frontend/src/metabase/dashboard/actions/parameters.ts index 74aa2331bfb0b3ff512be75536c50c445f3da2c3..11dd3fd3d3b16762d130865f9554f089c31e3f0e 100644 --- a/frontend/src/metabase/dashboard/actions/parameters.ts +++ b/frontend/src/metabase/dashboard/actions/parameters.ts @@ -31,6 +31,7 @@ import type { ParameterId, ParameterMappingOptions, ParameterTarget, + TemporalUnit, ValuesQueryType, ValuesSourceConfig, ValuesSourceType, @@ -487,6 +488,25 @@ export const setParameterIsMultiSelect = createThunkAction( }, ); +export const SET_PARAMETER_TEMPORAL_UNITS = + "metabase/dashboard/SET_PARAMETER_TEMPORAL_UNITS"; +export const setParameterTemporalUnits = createThunkAction( + SET_PARAMETER_TEMPORAL_UNITS, + (parameterId: ParameterId, temporalUnits: TemporalUnit[]) => + (dispatch, getState) => { + updateParameter(dispatch, getState, parameterId, parameter => ({ + ...parameter, + temporal_units: temporalUnits, + default: + parameter.default && temporalUnits.includes(parameter.default) + ? parameter.default + : undefined, + })); + + return { id: parameterId, temporalUnits }; + }, +); + export const SET_PARAMETER_QUERY_TYPE = "metabase/dashboard/SET_PARAMETER_QUERY_TYPE"; export const setParameterQueryType = createThunkAction( diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx index 688fcd7b552a00432fe19d93a87a49e750ef998b..705db432e997b3303ed9d9ec1b6c7a7f92c539e3 100644 --- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx +++ b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx @@ -118,6 +118,7 @@ export type DashboardProps = { navigateToNewCardFromDashboard: typeof navigateToNewCardFromDashboard; setParameterDefaultValue: (id: ParameterId, value: RowValue) => void; setParameterRequired: (id: ParameterId, value: boolean) => void; + setParameterTemporalUnits: (id: ParameterId, value: boolean) => void; setParameterIsMultiSelect: (id: ParameterId, isMultiSelect: boolean) => void; setParameterQueryType: (id: ParameterId, queryType: ValuesQueryType) => void; setParameterSourceType: ( @@ -543,6 +544,7 @@ function DashboardInner(props: DashboardProps) { props.setParameterFilteringParameters } setParameterRequired={props.setParameterRequired} + setParameterTemporalUnits={props.setParameterTemporalUnits} isFullscreen={props.isFullscreen} params={props.params} sidebar={props.sidebar} diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx index 7576b4a6b46bd585b106304f0e7719ab2c259a92..5e729a2214beeb4b9a59a3f21038083031ffa356 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx @@ -432,11 +432,10 @@ export const DashboardHeader = (props: DashboardHeaderProps) => { label={t`Add a Unit of Time widget`} > <DashboardHeaderButton + icon="clock" aria-label={t`Add a Unit of Time widget`} onClick={() => dispatch(addTemporalUnitParameter())} - > - <Icon name="clock" /> - </DashboardHeaderButton> + /> </Tooltip>, ); @@ -458,11 +457,10 @@ export const DashboardHeader = (props: DashboardHeaderProps) => { <Tooltip label={t`Add a filter`}> <DashboardHeaderButton key="parameters" + icon="filter" onClick={showAddParameterPopover} aria-label={t`Add a filter`} - > - <Icon name="filter" /> - </DashboardHeaderButton> + /> </Tooltip> </div> </TippyPopover> diff --git a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx index 921528d66273c932df4e06db80dd6a06844f8299..0aa511909b1bcab229f45d6e4f50ba65b92fc386 100644 --- a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx @@ -36,6 +36,7 @@ DashboardSidebars.propTypes = { setParameterSourceConfig: PropTypes.func.isRequired, setParameterFilteringParameters: PropTypes.func.isRequired, setParameterRequired: PropTypes.func.isRequired, + setParameterTemporalUnits: PropTypes.func.isRequired, isFullscreen: PropTypes.bool.isRequired, onCancel: PropTypes.func.isRequired, params: PropTypes.object, @@ -66,6 +67,7 @@ export function DashboardSidebars({ setParameterSourceConfig, setParameterFilteringParameters, setParameterRequired, + setParameterTemporalUnits, isFullscreen, onCancel, params, @@ -150,6 +152,7 @@ export function DashboardSidebars({ onShowAddParameterPopover={showAddParameterPopover} onClose={closeSidebar} onChangeRequired={setParameterRequired} + onChangeTemporalUnits={setParameterTemporalUnits} hasMapping={hasMapping(parameter, dashboard)} /> ); diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx index 6977799f6a1806774be595c7886e0c9fe33aed8b..a9b35884a67a75ce5fcdae4eec339ab750b536b6 100644 --- a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx @@ -19,10 +19,14 @@ import { } from "metabase/ui"; import type { ParameterSectionId } from "metabase-lib/v1/parameters/utils/operators"; import { canUseCustomSource } from "metabase-lib/v1/parameters/utils/parameter-source"; -import { isTemporalUnitParameter } from "metabase-lib/v1/parameters/utils/parameter-type"; +import { + isFilterParameter, + isTemporalUnitParameter, +} from "metabase-lib/v1/parameters/utils/parameter-type"; import { parameterHasNoDisplayValue } from "metabase-lib/v1/parameters/utils/parameter-values"; import type { Parameter, + TemporalUnit, ValuesQueryType, ValuesSourceConfig, ValuesSourceType, @@ -38,6 +42,7 @@ import { SettingLabelError, SettingValueWidget, } from "./ParameterSettings.styled"; +import { TemporalUnitSettings } from "./TemporalUnitSettings"; export interface ParameterSettingsProps { parameter: Parameter; @@ -51,6 +56,7 @@ export interface ParameterSettingsProps { onChangeSourceType: (sourceType: ValuesSourceType) => void; onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void; onChangeRequired: (value: boolean) => void; + onChangeTemporalUnits: (temporalUnits: TemporalUnit[]) => void; embeddedParameterVisibility: EmbeddingParameterVisibility | null; } @@ -72,6 +78,7 @@ export const ParameterSettings = ({ onChangeSourceType, onChangeSourceConfig, onChangeRequired, + onChangeTemporalUnits, embeddedParameterVisibility, hasMapping, }: ParameterSettingsProps): JSX.Element => { @@ -161,7 +168,7 @@ export const ParameterSettings = ({ aria-label={t`Label`} /> </Box> - {sectionId && !isTemporalUnitParameter(parameter) && ( + {sectionId && isFilterParameter(parameter) && ( <> <Box mb="xl"> <SettingLabel>{t`Filter type`}</SettingLabel> @@ -183,7 +190,15 @@ export const ParameterSettings = ({ )} </> )} - + {isTemporalUnitParameter(parameter) && ( + <Box mb="xl"> + <SettingLabel>{t`Unit of Time options`}</SettingLabel> + <TemporalUnitSettings + parameter={parameter} + onChangeTemporalUnits={onChangeTemporalUnits} + /> + </Box> + )} {canUseCustomSource(parameter) && ( <Box mb="xl"> <SettingLabel>{t`How should people filter on this column?`}</SettingLabel> diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx index 2bb6965c60e86572a5e450a821c542e57242936f..de1e61321438a43b403eb14bfebf9f1e1995c897 100644 --- a/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.unit.spec.tsx @@ -232,12 +232,33 @@ describe("ParameterSidebar", () => { expect(screen.getByDisplayValue("Equal to")).toBeInTheDocument(); }); }); + + describe("temporal-unit", () => { + it("should not render the type or operator", async () => { + setup({ parameter: createMockUiParameter({ type: "temporal-unit" }) }); + expect(await screen.findByText("Label")).toBeInTheDocument(); + expect(screen.queryByText("Filter type")).not.toBeInTheDocument(); + expect(screen.queryByText("Filter operator")).not.toBeInTheDocument(); + }); + + it("should set available temporal units", async () => { + const parameter = createMockUiParameter({ + type: "temporal-unit", + temporal_units: ["day"], + }); + const { onChangeTemporalUnits } = setup({ parameter }); + await userEvent.click(await screen.findByText("Day")); + await userEvent.click(await screen.findByLabelText("Month")); + expect(onChangeTemporalUnits).toHaveBeenCalledWith(["day", "month"]); + }); + }); }); const setup = ({ parameter = createMockUiParameter() }: SetupOpts = {}) => { const onChangeQueryType = jest.fn(); const onChangeName = jest.fn(); const onChangeIsMultiSelect = jest.fn(); + const onChangeTemporalUnits = jest.fn(); renderWithProviders( <ParameterSettings @@ -252,9 +273,15 @@ const setup = ({ parameter = createMockUiParameter() }: SetupOpts = {}) => { onChangeSourceType={jest.fn()} onChangeSourceConfig={jest.fn()} onChangeRequired={jest.fn()} + onChangeTemporalUnits={onChangeTemporalUnits} hasMapping={false} />, ); - return { onChangeQueryType, onChangeName, onChangeIsMultiSelect }; + return { + onChangeQueryType, + onChangeName, + onChangeIsMultiSelect, + onChangeTemporalUnits, + }; }; diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.module.css b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.module.css new file mode 100644 index 0000000000000000000000000000000000000000..448b51495aa169a1c1d7781dbb1bf4aa321fe669 --- /dev/null +++ b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.module.css @@ -0,0 +1,11 @@ +.label { + display: flex; + align-items: center; + cursor: pointer; + padding: 0.5rem; + border-radius: 0.375rem; + + &:hover { + background-color: var(--mb-color-bg-medium); + } +} diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.tsx new file mode 100644 index 0000000000000000000000000000000000000000..15ec667867955fb52d626d826b44bcc1627457a4 --- /dev/null +++ b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.tsx @@ -0,0 +1,147 @@ +import { t } from "ttag"; + +import { + Box, + Button, + Checkbox, + Divider, + Icon, + Popover, + Text, +} from "metabase/ui"; +import * as Lib from "metabase-lib"; +import type { Parameter, TemporalUnit } from "metabase-types/api"; + +import S from "./TemporalUnitSettings.module.css"; + +const VISIBLE_UNIT_LIMIT = 3; + +interface TemporalUnitSettingsProps { + parameter: Parameter; + onChangeTemporalUnits: (temporalUnits: TemporalUnit[]) => void; +} + +export function TemporalUnitSettings({ + parameter, + onChangeTemporalUnits, +}: TemporalUnitSettingsProps) { + const availableUnits = Lib.availableTemporalUnits(); + const selectedUnits = parameter.temporal_units ?? availableUnits; + const isAll = selectedUnits.length === availableUnits.length; + const isNone = selectedUnits.length === 0; + + return ( + <Popover width="target"> + <Popover.Target> + <Button + fw="normal" + rightIcon={<Icon name="chevrondown" />} + fullWidth + styles={{ inner: { justifyContent: "space-between" } }} // justify prop in mantine v7 + > + {getSelectedText(selectedUnits, isAll, isNone)} + </Button> + </Popover.Target> + <Popover.Dropdown> + <TemporalUnitDropdown + selectedUnits={selectedUnits} + availableUnits={availableUnits} + isAll={isAll} + isNone={isNone} + onChange={onChangeTemporalUnits} + /> + </Popover.Dropdown> + </Popover> + ); +} + +interface TemporalUnitDropdownProps { + selectedUnits: TemporalUnit[]; + availableUnits: TemporalUnit[]; + isAll: boolean; + isNone: boolean; + onChange: (selectedUnits: TemporalUnit[]) => void; +} + +function TemporalUnitDropdown({ + selectedUnits, + availableUnits, + isAll, + isNone, + onChange, +}: TemporalUnitDropdownProps) { + const selectedUnitsSet = new Set(selectedUnits); + const isDisabledDeselection = selectedUnits.length <= 1; + + const handleAllToggle = () => { + if (isAll) { + onChange([availableUnits[0]]); + } else { + onChange(availableUnits); + } + }; + + const handleUnitToggle = (selectedUnit: TemporalUnit) => { + const newSelectedUnits = availableUnits.filter(availableUnit => { + if (availableUnit === selectedUnit) { + return !selectedUnitsSet.has(selectedUnit); + } else { + return selectedUnitsSet.has(availableUnit); + } + }); + + onChange(newSelectedUnits); + }; + + return ( + <Box p="sm"> + <label className={S.label}> + <Checkbox + checked={isAll} + indeterminate={!isAll && !isNone} + variant="stacked" + onChange={handleAllToggle} + /> + <Text ml="sm">{isAll ? t`Select none` : t`Select all`}</Text> + </label> + <Divider /> + {availableUnits.map(unit => { + const isSelected = selectedUnitsSet.has(unit); + const isDisabled = isSelected && isDisabledDeselection; + + return ( + <label key={unit} className={S.label}> + <Checkbox + checked={isSelected} + disabled={isDisabled} + onChange={() => handleUnitToggle(unit)} + /> + <Text ml="sm">{Lib.describeTemporalUnit(unit)}</Text> + </label> + ); + })} + </Box> + ); +} + +function getSelectedText( + selectedUnits: TemporalUnit[], + isAll: boolean, + isNone: boolean, +) { + if (isAll) { + return t`All`; + } + if (isNone) { + return t`None`; + } + + const visibleUnitCount = Math.min(selectedUnits.length, VISIBLE_UNIT_LIMIT); + const hiddenUnitCount = selectedUnits.length - visibleUnitCount; + + return selectedUnits + .slice(0, visibleUnitCount) + .map(unit => Lib.describeTemporalUnit(unit)) + .concat(hiddenUnitCount > 0 ? [`+${hiddenUnitCount}`] : []) + .join(", "); +} diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..2839583be81fe138ef08717f31ccedfa7abe7a3c --- /dev/null +++ b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/TemporalUnitSettings.unit.spec.tsx @@ -0,0 +1,118 @@ +import userEvent from "@testing-library/user-event"; + +import { render, screen } from "__support__/ui"; +import * as Lib from "metabase-lib"; +import type { Parameter, TemporalUnit } from "metabase-types/api"; +import { createMockParameter } from "metabase-types/api/mocks"; + +import { TemporalUnitSettings } from "./TemporalUnitSettings"; + +interface SetupOpts { + parameter?: Parameter; +} + +function setup({ parameter = createMockParameter() }: SetupOpts = {}) { + const onChangeTemporalUnits = jest.fn(); + + render( + <TemporalUnitSettings + parameter={parameter} + onChangeTemporalUnits={onChangeTemporalUnits} + />, + ); + + return { onChangeTemporalUnits }; +} + +describe("TemporalUnitSettings", () => { + it.each<[string, TemporalUnit[] | undefined]>([ + ["All", undefined], + ["All", Lib.availableTemporalUnits()], + ["None", []], + ["Minute", ["minute"]], + ["Month, Quarter", ["month", "quarter"]], + ["Month, Quarter, Day of week", ["month", "quarter", "day-of-week"]], + ["Day, Month, Quarter, +1", ["day", "month", "quarter", "year"]], + ["Hour, Day, Month, +2", ["hour", "day", "month", "quarter", "year"]], + ])("should correctly format %s", async (value, units) => { + setup({ + parameter: createMockParameter({ + type: "temporal-unit", + temporal_units: units, + }), + }); + + expect(await screen.findByText(value)).toBeInTheDocument(); + }); + + it("should select a unit and preserve the unit order", async () => { + const { onChangeTemporalUnits } = setup({ + parameter: createMockParameter({ + type: "temporal-unit", + temporal_units: ["day", "year"], + }), + }); + await userEvent.click(await screen.findByText("Day, Year")); + await userEvent.click(await screen.findByLabelText("Month")); + expect(onChangeTemporalUnits).toHaveBeenCalledWith([ + "day", + "month", + "year", + ]); + }); + + it("should deselect a unit and preserve the unit order", async () => { + const { onChangeTemporalUnits } = setup({ + parameter: createMockParameter({ + type: "temporal-unit", + temporal_units: ["day", "month", "quarter", "year"], + }), + }); + await userEvent.click(await screen.findByText("Day, Month, Quarter, +1")); + await userEvent.click(await screen.findByLabelText("Quarter")); + expect(onChangeTemporalUnits).toHaveBeenCalledWith([ + "day", + "month", + "year", + ]); + }); + + it("should select all the units when only 1 was selected", async () => { + const { onChangeTemporalUnits } = setup({ + parameter: createMockParameter({ + type: "temporal-unit", + temporal_units: ["minute"], + }), + }); + await userEvent.click(await screen.findByText("Minute")); + await userEvent.click(await screen.findByLabelText("Select all")); + expect(onChangeTemporalUnits).toHaveBeenCalledWith( + Lib.availableTemporalUnits(), + ); + }); + + it("should select all the units when none was selected", async () => { + const { onChangeTemporalUnits } = setup({ + parameter: createMockParameter({ + type: "temporal-unit", + temporal_units: [], + }), + }); + await userEvent.click(await screen.findByText("None")); + await userEvent.click(await screen.findByLabelText("Select all")); + expect(onChangeTemporalUnits).toHaveBeenCalledWith( + Lib.availableTemporalUnits(), + ); + }); + + it("should preserve 1 unit when deselecting all the units", async () => { + const { onChangeTemporalUnits } = setup({ + parameter: createMockParameter({ + type: "temporal-unit", + }), + }); + await userEvent.click(await screen.findByText("All")); + await userEvent.click(await screen.findByLabelText("Select none")); + expect(onChangeTemporalUnits).toHaveBeenCalledWith(["minute"]); + }); +}); diff --git a/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/index.ts b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..7fd71b27475b7394676d1159efc5740b2ab8f23b --- /dev/null +++ b/frontend/src/metabase/parameters/components/ParameterSettings/TemporalUnitSettings/index.ts @@ -0,0 +1 @@ +export * from "./TemporalUnitSettings"; diff --git a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx index b7f935e15a4eeb9be6df6dac8c6a130f61201e46..65cd8750056dd5009d1e6356a244708d2bfe8fde 100644 --- a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.tsx @@ -13,6 +13,7 @@ import { parameterHasNoDisplayValue } from "metabase-lib/v1/parameters/utils/par import type { Parameter, ParameterId, + TemporalUnit, ValuesQueryType, ValuesSourceConfig, ValuesSourceType, @@ -54,6 +55,10 @@ export interface ParameterSidebarProps { filteringParameters: string[], ) => void; onChangeRequired: (parameterId: ParameterId, value: boolean) => void; + onChangeTemporalUnits: ( + parameterId: ParameterId, + temporalUnits: TemporalUnit[], + ) => void; onRemoveParameter: (parameterId: ParameterId) => void; onShowAddParameterPopover: () => void; onClose: () => void; @@ -71,6 +76,7 @@ export const ParameterSidebar = ({ onChangeSourceConfig, onChangeFilteringParameters, onChangeRequired, + onChangeTemporalUnits, onRemoveParameter, onShowAddParameterPopover, onClose, @@ -164,6 +170,9 @@ export const ParameterSidebar = ({ const handleChangeRequired = (value: boolean) => onChangeRequired(parameterId, value); + const handleChangeTemporalUnits = (temporalUnits: TemporalUnit[]) => + onChangeTemporalUnits(parameterId, temporalUnits); + const handleTabChange = (newTab: string | null) => { if (!newTab || (newTab !== "settings" && newTab !== "filters")) { return; @@ -229,6 +238,7 @@ export const ParameterSidebar = ({ onChangeSourceType={handleSourceTypeChange} onChangeSourceConfig={handleSourceConfigChange} onChangeRequired={handleChangeRequired} + onChangeTemporalUnits={handleChangeTemporalUnits} hasMapping={hasMapping} /> </Tabs.Panel> diff --git a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx index 9a86ad11977dd3e9f2608f4f3759a06abb39f02b..0b4732b32a0e0cbc7c5617d0ff95942cd29a7ce4 100644 --- a/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx +++ b/frontend/src/metabase/parameters/components/ParameterSidebar/ParameterSidebar.unit.spec.tsx @@ -56,6 +56,7 @@ const setup = ({ onChangeSourceConfig={jest.fn()} onChangeFilteringParameters={jest.fn()} onRemoveParameter={jest.fn()} + onChangeTemporalUnits={jest.fn()} onShowAddParameterPopover={jest.fn()} onClose={jest.fn()} onChangeRequired={jest.fn()} diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx index cd5879c80c8bce9cf7c9e7e4628762b2503fcbad..dba6f7de29ef7ce93c79b1dd2e291141eda183d9 100644 --- a/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx +++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.jsx @@ -323,7 +323,14 @@ function Widget({ } if (isTemporalUnitParameter(parameter)) { - return <TemporalUnitWidget setValue={setValue} onClose={onPopoverClose} />; + return ( + <TemporalUnitWidget + parameter={parameter} + value={value} + setValue={setValue} + onClose={onPopoverClose} + /> + ); } if (isTextWidget(parameter)) { diff --git a/frontend/src/metabase/parameters/components/widgets/TemporalUnitWidget/TemporalUnitWidget.tsx b/frontend/src/metabase/parameters/components/widgets/TemporalUnitWidget/TemporalUnitWidget.tsx index 96a3428041178a21376f8e63f565a16dcad6c864..ffb79b5b8d3be82fc0b88b85671b1219f4bb4049 100644 --- a/frontend/src/metabase/parameters/components/widgets/TemporalUnitWidget/TemporalUnitWidget.tsx +++ b/frontend/src/metabase/parameters/components/widgets/TemporalUnitWidget/TemporalUnitWidget.tsx @@ -1,36 +1,39 @@ -import { Box, Button } from "metabase/ui"; +import { Box, SelectItem } from "metabase/ui"; import * as Lib from "metabase-lib"; +import type { Parameter, TemporalUnit } from "metabase-types/api"; const MIN_WIDTH = 180; interface TemporalUnitWidgetProps { - setValue: (unit: string) => void; + parameter: Parameter; + value?: TemporalUnit; + setValue: (unit: TemporalUnit) => void; onClose: () => void; } export function TemporalUnitWidget({ + parameter, + value, setValue, onClose, }: TemporalUnitWidgetProps) { - const units = Lib.availableTemporalUnits(); + const availableUnits = + parameter.temporal_units ?? Lib.availableTemporalUnits(); - const handleSelect = (unit: string) => { + const handleSelect = (unit: TemporalUnit) => { setValue(unit); onClose(); }; return ( <Box p="sm" miw={MIN_WIDTH}> - {units.map(unit => ( - <Button + {availableUnits.map(unit => ( + <SelectItem key={unit} - c="text-dark" - display="block" - variant="subtle" + value={Lib.describeTemporalUnit(unit)} + selected={value === unit} onClick={() => handleSelect(unit)} - > - {Lib.describeTemporalUnit(unit)} - </Button> + /> ))} </Box> ); diff --git a/frontend/src/metabase/parameters/components/widgets/TemporalUnitWidget/TemporalUnitWidget.unit.spec.tsx b/frontend/src/metabase/parameters/components/widgets/TemporalUnitWidget/TemporalUnitWidget.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..7858367e10074846da5a229f1597ff0883b7450c --- /dev/null +++ b/frontend/src/metabase/parameters/components/widgets/TemporalUnitWidget/TemporalUnitWidget.unit.spec.tsx @@ -0,0 +1,75 @@ +import userEvent from "@testing-library/user-event"; + +import { render, screen } from "__support__/ui"; +import type { Parameter, TemporalUnit } from "metabase-types/api"; +import { createMockParameter } from "metabase-types/api/mocks"; + +import { TemporalUnitWidget } from "./TemporalUnitWidget"; + +interface SetupOpts { + parameter?: Parameter; + value?: TemporalUnit; +} + +function setup({ + parameter = createMockParameter({ type: "temporal-unit" }), + value, +}: SetupOpts = {}) { + const setValue = jest.fn(); + const onClose = jest.fn(); + + render( + <TemporalUnitWidget + parameter={parameter} + value={value} + setValue={setValue} + onClose={onClose} + />, + ); + + return { setValue, onClose }; +} + +describe("TemporalUnitWidget", () => { + it("should show all temporal units by default", () => { + setup(); + + expect(screen.getByRole("option", { name: "Day" })).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "Month" })).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "Year" })).toBeInTheDocument(); + }); + + it("should show only the allowed temporal units when specified", () => { + setup({ + parameter: createMockParameter({ + type: "temporal-unit", + temporal_units: ["month", "year"], + }), + }); + + expect(screen.getByRole("option", { name: "Month" })).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "Year" })).toBeInTheDocument(); + expect( + screen.queryByRole("option", { name: "Day" }), + ).not.toBeInTheDocument(); + }); + + it("should highlight the selected item", () => { + setup({ value: "month" }); + expect(screen.getByRole("option", { name: "Month" })).toHaveAttribute( + "aria-selected", + "true", + ); + expect(screen.getByRole("option", { name: "Year" })).not.toHaveAttribute( + "aria-selected", + "true", + ); + }); + + it("should select a temporal unit and close the popover", async () => { + const { setValue, onClose } = setup(); + await userEvent.click(screen.getByRole("option", { name: "Month" })); + expect(setValue).toHaveBeenCalledWith("month"); + expect(onClose).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/metabase/querying/components/FilterValuePicker/ListValuePicker/utils.ts b/frontend/src/metabase/querying/components/FilterValuePicker/ListValuePicker/utils.ts index 1ac6c62f17962677d14d0bd34f0153f6afbb41c8..61b7440ea8accc0739edbbced11b1d94ad6f2490 100644 --- a/frontend/src/metabase/querying/components/FilterValuePicker/ListValuePicker/utils.ts +++ b/frontend/src/metabase/querying/components/FilterValuePicker/ListValuePicker/utils.ts @@ -1,9 +1,9 @@ -import type { SelectItem } from "metabase/ui"; +import type { SelectOption } from "metabase/ui"; export function searchOptions( - options: SelectItem[], + options: SelectOption[], searchText: string, -): SelectItem[] { +): SelectOption[] { const searchValue = searchText.toLowerCase(); return options.filter( ({ label }) => label != null && label.toLowerCase().includes(searchValue), diff --git a/frontend/src/metabase/querying/components/FilterValuePicker/utils.ts b/frontend/src/metabase/querying/components/FilterValuePicker/utils.ts index fe4e91bd5c369cf63316e655e899517dd7ddbc1e..acbbb8174c02b879672638d420824f22fbb3704b 100644 --- a/frontend/src/metabase/querying/components/FilterValuePicker/utils.ts +++ b/frontend/src/metabase/querying/components/FilterValuePicker/utils.ts @@ -1,4 +1,4 @@ -import type { SelectItem } from "metabase/ui"; +import type { SelectOption } from "metabase/ui"; import type { FieldValuesSearchInfo } from "metabase-lib"; import * as Lib from "metabase-lib"; import type { FieldValue, GetFieldValuesResponse } from "metabase-types/api"; @@ -29,7 +29,7 @@ export function canSearchFieldValues( ); } -export function getFieldOptions(fieldValues: FieldValue[]): SelectItem[] { +export function getFieldOptions(fieldValues: FieldValue[]): SelectOption[] { return fieldValues .filter(([value]) => value != null) .map(([value, label = value]) => ({ @@ -38,7 +38,7 @@ export function getFieldOptions(fieldValues: FieldValue[]): SelectItem[] { })); } -function getSelectedOptions(selectedValues: string[]): SelectItem[] { +function getSelectedOptions(selectedValues: string[]): SelectOption[] { return selectedValues.map(value => ({ value, })); @@ -48,7 +48,7 @@ export function getEffectiveOptions( fieldValues: FieldValue[], selectedValues: string[], elevatedValues: string[] = [], -): SelectItem[] { +): SelectOption[] { const options = [ ...getSelectedOptions(elevatedValues), ...getFieldOptions(fieldValues), diff --git a/frontend/src/metabase/ui/components/inputs/Select/Select.styled.tsx b/frontend/src/metabase/ui/components/inputs/Select/Select.styled.tsx index 22d9474354d798ea7ffc75aa226ae1d9d46cc413..d8b2ac60c51f974ede5716834b4a1b23eab791e9 100644 --- a/frontend/src/metabase/ui/components/inputs/Select/Select.styled.tsx +++ b/frontend/src/metabase/ui/components/inputs/Select/Select.styled.tsx @@ -1,13 +1,16 @@ -import type { - MantineSize, - MantineTheme, - MantineThemeOverride, - CSSObject, +import { + type CSSObject, + type MantineSize, + type MantineTheme, + type MantineThemeOverride, + getSize, + getStylesRef, + rem, + px, } from "@mantine/core"; -import { getStylesRef, px, rem, getSize } from "@mantine/core"; import { SelectDropdown } from "./SelectDropdown"; -import { SelectItem } from "./SelectItem"; +import { SelectItem, getItemFontSize, getItemLineHeight } from "./SelectItem"; export const getSelectOverrides = (): MantineThemeOverride["components"] => ({ Select: { @@ -109,16 +112,6 @@ export const getSelectInputOverrides = ( }; }; -const LINE_HEIGHTS = { - xs: rem(16), - md: rem(24), -}; - -const ITEM_FONT_SIZES = { - xs: rem(12), - md: rem(14), -}; - const SEPARATOR_FONT_SIZES = { xs: rem(12), md: rem(12), @@ -134,8 +127,8 @@ export const getSelectItemsOverrides = ( }, item: { color: theme.fn.themeColor("text-dark"), - fontSize: getSize({ size, sizes: ITEM_FONT_SIZES }), - lineHeight: getSize({ size, sizes: LINE_HEIGHTS }), + fontSize: getItemFontSize(size), + lineHeight: getItemLineHeight(size), padding: theme.spacing.sm, "&[data-hovered]": { color: theme.fn.themeColor("brand"), @@ -175,8 +168,8 @@ export const getSelectItemsOverrides = ( }, nothingFound: { color: theme.fn.themeColor("text-light"), - fontSize: getSize({ size, sizes: ITEM_FONT_SIZES }), - lineHeight: getSize({ size, sizes: LINE_HEIGHTS }), + fontSize: getItemFontSize(size), + lineHeight: getItemLineHeight(size), padding: theme.spacing.sm, }, }; diff --git a/frontend/src/metabase/ui/components/inputs/Select/SelectItem/SelectItem.module.css b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/SelectItem.module.css new file mode 100644 index 0000000000000000000000000000000000000000..3d3b085e6daa006eeaab28638310de8018069b6e --- /dev/null +++ b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/SelectItem.module.css @@ -0,0 +1,20 @@ +.item { + cursor: pointer; + border-radius: 6px; + word-break: break-all; + + &:hover { + color: var(--mb-color-brand); + background-color: var(--mb-color-brand-lighter); + } + + &.selected { + color: var(--mb-color-text-white); + background-color: var(--mb-color-brand); + } + + &.disabled { + cursor: default; + color: var(--mb-color-text-light); + } +} diff --git a/frontend/src/metabase/ui/components/inputs/Select/SelectItem/SelectItem.tsx b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/SelectItem.tsx index 34c8c6c8f186fa0a1ed76be41d97665cf95be708..9c300a49572d435374eebffb23be8b468cc130c2 100644 --- a/frontend/src/metabase/ui/components/inputs/Select/SelectItem/SelectItem.tsx +++ b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/SelectItem.tsx @@ -1,22 +1,54 @@ -import { Group, Text } from "@mantine/core"; -import type { Ref, HTMLAttributes } from "react"; -import { forwardRef } from "react"; +import { type BoxProps, Group, type MantineSize, Text } from "@mantine/core"; +import cx from "classnames"; +import { forwardRef, type HTMLAttributes, type Ref } from "react"; -import type { IconName } from "metabase/ui"; -import { Icon } from "metabase/ui"; +import { Icon, type IconName } from "metabase/ui"; -interface SelectItemProps extends HTMLAttributes<HTMLDivElement> { +import S from "./SelectItem.module.css"; +import { getItemFontSize, getItemLineHeight } from "./utils"; + +interface SelectItemProps extends HTMLAttributes<HTMLDivElement>, BoxProps { value: string; label?: string; + size?: MantineSize; icon?: IconName; + selected?: boolean; + disabled?: boolean; } export const SelectItem = forwardRef(function SelectItem( - { value, label = value, icon, ...others }: SelectItemProps, + { + className, + value, + label = value, + size = "md", + icon, + selected, + disabled, + ...props + }: SelectItemProps, ref: Ref<HTMLDivElement>, ) { return ( - <Group ref={ref} spacing="sm" {...others}> + <Group + ref={ref} + className={cx( + S.item, + { + [S.selected]: selected, + [S.disabled]: disabled, + }, + className, + )} + color="text-dark" + fz={getItemFontSize(size)} + lh={getItemLineHeight(size)} + p="sm" + spacing="sm" + role="option" + aria-selected={selected} + {...props} + > {icon && <Icon name={icon} />} <Text color="inherit" lh="inherit"> {label} diff --git a/frontend/src/metabase/ui/components/inputs/Select/SelectItem/index.ts b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/index.ts index 4d294a8bfa5a9d1fe5cdd112669bc9e1d42109b4..7e454fd07d42f6a363d0e097b8a10e1db811e711 100644 --- a/frontend/src/metabase/ui/components/inputs/Select/SelectItem/index.ts +++ b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/index.ts @@ -1 +1,2 @@ export * from "./SelectItem"; +export * from "./utils"; diff --git a/frontend/src/metabase/ui/components/inputs/Select/SelectItem/utils.ts b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/utils.ts new file mode 100644 index 0000000000000000000000000000000000000000..37839a0df640b838c5a95ab45c9e83b2cc7a25ff --- /dev/null +++ b/frontend/src/metabase/ui/components/inputs/Select/SelectItem/utils.ts @@ -0,0 +1,19 @@ +import { getSize, type MantineSize, rem } from "@mantine/core"; + +const FONT_SIZES = { + xs: rem(12), + md: rem(14), +}; + +const LINE_HEIGHTS = { + xs: rem(16), + md: rem(24), +}; + +export function getItemFontSize(size: MantineSize | number): string { + return getSize({ size, sizes: FONT_SIZES }); +} + +export function getItemLineHeight(size: MantineSize | number): string { + return getSize({ size, sizes: LINE_HEIGHTS }); +} diff --git a/frontend/src/metabase/ui/components/inputs/Select/index.ts b/frontend/src/metabase/ui/components/inputs/Select/index.ts index d19c489ee107a8400f8338e8e75d8479b9deaf83..720b1721bebbea59cc9e1aa770aed050d7c86254 100644 --- a/frontend/src/metabase/ui/components/inputs/Select/index.ts +++ b/frontend/src/metabase/ui/components/inputs/Select/index.ts @@ -1,3 +1,4 @@ export { Select } from "@mantine/core"; -export type { SelectProps, SelectItem } from "@mantine/core"; +export type { SelectProps, SelectItem as SelectOption } from "@mantine/core"; +export { SelectItem } from "./SelectItem"; export { getSelectOverrides } from "./Select.styled";