diff --git a/e2e/support/helpers/e2e-custom-column-helpers.js b/e2e/support/helpers/e2e-custom-column-helpers.js index 8387bdc8400091f62f169a5434e285ed69c648e6..f196662b49deeae5b26f52bf35d16e2ba2a69493 100644 --- a/e2e/support/helpers/e2e-custom-column-helpers.js +++ b/e2e/support/helpers/e2e-custom-column-helpers.js @@ -1,3 +1,7 @@ +export function expressionEditorWidget() { + return cy.findByTestId("expression-editor"); +} + export function enterCustomColumnDetails({ formula, name } = {}) { cy.get(".ace_text-input") .first() diff --git a/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js b/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js index bf80d32c532a20a35f1c572df5eb50d117a414e3..1b01a418c41a0ca0f01c0027f1f9bf4c49a7e508 100644 --- a/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js +++ b/e2e/test/scenarios/custom-column/reproductions/13751-cc-allow-strings-in-filter.cy.spec.js @@ -26,13 +26,11 @@ describe("issue 13751", { tags: "@external" }, () => { it("should allow using strings in filter based on a custom column (metabase#13751)", () => { addCustomColumn(); - popover().within(() => { - enterCustomColumnDetails({ - formula: 'regexextract([State], "^C[A-Z]")', - }); - cy.findByPlaceholderText("Something nice and descriptive").type(CC_NAME); - cy.button("Done").click(); + enterCustomColumnDetails({ + formula: 'regexextract([State], "^C[A-Z]")', + name: CC_NAME, }); + cy.button("Done").click(); getNotebookStep("filter") .findByText(/Add filter/) diff --git a/e2e/test/scenarios/filters/filter.cy.spec.js b/e2e/test/scenarios/filters/filter.cy.spec.js index db8cadd58f19d149dd27a6923e812262b0be4a4f..41f807aa169e0b020afe6ed687c763c95f4e8562 100644 --- a/e2e/test/scenarios/filters/filter.cy.spec.js +++ b/e2e/test/scenarios/filters/filter.cy.spec.js @@ -21,6 +21,7 @@ import { getNotebookStep, queryBuilderMain, selectFilterOperator, + expressionEditorWidget, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID, REVIEWS, REVIEWS_ID } = @@ -972,7 +973,7 @@ describe("scenarios > question > filter", () => { filter({ mode: "notebook" }); popover().contains("Custom Expression").click(); - popover().within(() => { + expressionEditorWidget().within(() => { enterCustomColumnDetails({ formula: `boolean = ${condition}` }); cy.get("@formula").blur(); @@ -1021,7 +1022,7 @@ describe("scenarios > question > filter", () => { filter({ mode: "notebook" }); popover().contains("Custom Expression").click(); - popover().within(() => { + expressionEditorWidget().within(() => { enterCustomColumnDetails({ formula: "boolean = true" }); cy.get("@formula").blur(); @@ -1038,7 +1039,7 @@ describe("scenarios > question > filter", () => { cy.icon("notebook").click(); summarize({ mode: "notebook" }); popover().contains("Custom Expression").click(); - popover().within(() => { + expressionEditorWidget().within(() => { enterCustomColumnDetails({ formula: "CountIf(boolean)", name: "count if boolean is true", @@ -1059,7 +1060,7 @@ describe("scenarios > question > filter", () => { openReviewsTable({ mode: "notebook" }); filter({ mode: "notebook" }); - popover().within(() => { + expressionEditorWidget().within(() => { cy.findByText("Custom Expression").click(); enterCustomColumnDetails({ formula: "floor" }); diff --git a/e2e/test/scenarios/question/reproductions/17512.cy.spec.js b/e2e/test/scenarios/question/reproductions/17512.cy.spec.js index ad637a33db8c52658c3ac841b9cf5249bbaf7e91..d8841e91fd7d829503dd651cea701b39ab6c6594 100644 --- a/e2e/test/scenarios/question/reproductions/17512.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/17512.cy.spec.js @@ -4,6 +4,8 @@ import { popover, visualize, summarize, + expressionEditorWidget, + enterCustomColumnDetails, } from "e2e/support/helpers"; describe("issue 17512", () => { @@ -44,18 +46,22 @@ function addSummarizeCustomExpression(formula, name) { summarize({ mode: "notebook" }); popover().contains("Custom Expression").click(); - popover().within(() => { - cy.get(".ace_text-input").type(formula).blur(); - cy.findByPlaceholderText("Something nice and descriptive").type(name); + expressionEditorWidget().within(() => { + enterCustomColumnDetails({ + formula, + name, + }); cy.button("Done").click(); }); } function addCustomColumn(formula, name) { cy.findByText("Custom column").click(); - popover().within(() => { - cy.get(".ace_text-input").type(formula).blur(); - cy.findByPlaceholderText("Something nice and descriptive").type(name); + expressionEditorWidget().within(() => { + enterCustomColumnDetails({ + formula, + name, + }); cy.button("Done").click(); }); } diff --git a/e2e/test/scenarios/question/reproductions/18207-string-min-max.cy.spec.js b/e2e/test/scenarios/question/reproductions/18207-string-min-max.cy.spec.js index 84eebc4ae18319096eee2c655ad2cefb30b58358..2c1b1b14f8bc13349ab071e7ffccfe09ee167f5c 100644 --- a/e2e/test/scenarios/question/reproductions/18207-string-min-max.cy.spec.js +++ b/e2e/test/scenarios/question/reproductions/18207-string-min-max.cy.spec.js @@ -6,6 +6,7 @@ import { openProductsTable, summarize, leftSidebar, + expressionEditorWidget, } from "e2e/support/helpers"; describe("issue 18207", () => { @@ -70,7 +71,7 @@ describe("issue 18207", () => { it("should be possible to group by a string expression (metabase#18207)", () => { popover().contains("Custom Expression").click(); - popover().within(() => { + expressionEditorWidget().within(() => { enterCustomColumnDetails({ formula: "Max([Vendor])", name: "LastVendor", diff --git a/e2e/test/scenarios/question/summarization.cy.spec.js b/e2e/test/scenarios/question/summarization.cy.spec.js index 443a87061a0791074a1bb298d34938819e2ac926..13f1a2319986442e1a9ae05865286f8e92e34794 100644 --- a/e2e/test/scenarios/question/summarization.cy.spec.js +++ b/e2e/test/scenarios/question/summarization.cy.spec.js @@ -15,6 +15,7 @@ import { checkExpressionEditorHelperPopoverPosition, rightSidebar, interceptIfNotPreviouslyDefined, + expressionEditorWidget, } from "e2e/support/helpers"; const { ORDERS, ORDERS_ID } = SAMPLE_DATABASE; @@ -156,7 +157,7 @@ describe("scenarios > question > summarize sidebar", () => { openOrdersTable({ mode: "notebook" }); summarize({ mode: "notebook" }); popover().contains("Custom Expression").click(); - popover().within(() => { + expressionEditorWidget().within(() => { enterCustomColumnDetails({ formula: "2 * Max([Total])", name: "twice max total", @@ -178,7 +179,7 @@ describe("scenarios > question > summarize sidebar", () => { summarize({ mode: "notebook" }); popover().contains("Custom Expression").click(); - popover().within(() => { + expressionEditorWidget().within(() => { enterCustomColumnDetails({ formula: "sum([Total]) / (sum([Product → Price]) * average([Quantity]))", diff --git a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js index 5e507a9947ab925b0d4a5a8886454c2dba2974ae..ac5fbbafc5a99d1909bce26c157fccc0b7abe581 100644 --- a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js +++ b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js @@ -19,6 +19,7 @@ import { sidebar, moveDnDKitElement, selectFilterOperator, + expressionEditorWidget, } from "e2e/support/helpers"; describe("scenarios > visualizations > table", () => { @@ -130,7 +131,7 @@ describe("scenarios > visualizations > table", () => { cy.icon("add_data").click(); - popover().within(() => { + expressionEditorWidget().within(() => { enterCustomColumnDetails({ formula: "concat([Name], [Name])", name: ccName, diff --git a/frontend/src/metabase-lib/v1/expressions/config.ts b/frontend/src/metabase-lib/v1/expressions/config.ts index 84f461e46a394a74ba32766c31a9a3581ea08a31..6816bf2a360f23ac0daf03849fa986bfaf245d7a 100644 --- a/frontend/src/metabase-lib/v1/expressions/config.ts +++ b/frontend/src/metabase-lib/v1/expressions/config.ts @@ -559,3 +559,28 @@ export const STANDARD_AGGREGATIONS = new Set([ "max", "median", ]); + +export const POPULAR_FUNCTIONS = [ + "case", + "concat", + "contains", + "between", + "coalesce", +]; + +export const POPULAR_FILTERS = [ + "contains", + "case", + "between", + "interval", + "concat", + "round", +]; + +export const POPULAR_AGGREGATIONS = [ + "count", + "distinct", + "count-where", + "sum", + "avg", +]; diff --git a/frontend/src/metabase-lib/v1/expressions/suggest.ts b/frontend/src/metabase-lib/v1/expressions/suggest.ts index 35f153b4a0ac60ff8e1ea5467a052eeceebc6ac1..b2399219024fb205915ea25138bea5efacb99abe 100644 --- a/frontend/src/metabase-lib/v1/expressions/suggest.ts +++ b/frontend/src/metabase-lib/v1/expressions/suggest.ts @@ -1,3 +1,4 @@ +import { t } from "ttag"; import _ from "underscore"; import * as Lib from "metabase-lib"; @@ -11,6 +12,9 @@ import { EXPRESSION_FUNCTIONS, getMBQLName, MBQL_CLAUSES, + POPULAR_AGGREGATIONS, + POPULAR_FILTERS, + POPULAR_FUNCTIONS, } from "metabase-lib/v1/expressions/config"; import { getHelpText } from "metabase-lib/v1/expressions/helper-text-strings"; import type { @@ -32,6 +36,7 @@ export type Suggestion = { range?: [number, number]; column?: Lib.ColumnMetadata; helpText?: HelpText; + group?: GroupName; }; const suggestionText = (func: MBQLClauseFunctionConfig) => { @@ -40,7 +45,18 @@ const suggestionText = (func: MBQLClauseFunctionConfig) => { return displayName + suffix; }; -type SuggestArgs = { +export const GROUPS = { + popularExpressions: { + displayName: t`Most used functions`, + }, + popularAggregations: { + displayName: t`Most used aggregations`, + }, +} as const; + +export type GroupName = keyof typeof GROUPS; + +export type SuggestArgs = { source: string; query: Lib.Query; stageIndex: number; @@ -90,6 +106,55 @@ export function suggest({ } } } + + if (source === "") { + let popular: string[] = []; + if (startRule === "expression") { + popular = POPULAR_FUNCTIONS; + } + if (startRule === "boolean") { + popular = POPULAR_FILTERS; + } + if (startRule === "aggregation") { + popular = POPULAR_AGGREGATIONS; + } + + suggestions.push( + ...popular + .map((name: string): Suggestion | null => { + const clause = MBQL_CLAUSES[name]; + if (!clause) { + return null; + } + + const isSupported = + !database || database?.hasFeature(clause.requiresFeature); + + if (!isSupported) { + return null; + } + + return { + type: "functions", + name: clause.displayName, + text: suggestionText(clause), + index: targetOffset, + icon: "function", + order: 1, + group: + startRule === "aggregation" + ? "popularAggregations" + : "popularExpressions", + helpText: database + ? getHelpText(name, database, reportTimezone) + : undefined, + }; + }) + .filter((suggestion): suggestion is Suggestion => Boolean(suggestion)) + .slice(0, 5), + ); + } + return { suggestions }; } diff --git a/frontend/src/metabase-lib/v1/expressions/suggest.unit.spec.ts b/frontend/src/metabase-lib/v1/expressions/suggest.unit.spec.ts index 8c8393d42c378f1f8f7313e11bb2a04ae8ed1203..afac642258996add3157103ba208110d3ce446cb 100644 --- a/frontend/src/metabase-lib/v1/expressions/suggest.unit.spec.ts +++ b/frontend/src/metabase-lib/v1/expressions/suggest.unit.spec.ts @@ -557,6 +557,116 @@ describe("metabase/lib/expression/suggest", () => { }), ]); }); + + it("should add suggestions for popular functions when no input is given", () => { + expect( + suggest_({ + source: "", + ...expressionOpts, + startRule: "expression", + query: createQuery({ + metadata, + query: DEFAULT_QUERY, + }), + stageIndex: -1, + metadata, + getColumnIcon: () => "icon", + }).suggestions, + ).toEqual([ + expect.objectContaining({ + name: "case", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "concat", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "contains", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "between", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "coalesce", + group: "popularExpressions", + }), + ]); + + expect( + suggest_({ + source: "", + ...expressionOpts, + startRule: "boolean", + query: createQuery({ + metadata, + query: DEFAULT_QUERY, + }), + stageIndex: -1, + metadata, + getColumnIcon: () => "icon", + }).suggestions, + ).toEqual([ + expect.objectContaining({ + name: "contains", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "case", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "between", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "timeSpan", + group: "popularExpressions", + }), + expect.objectContaining({ + name: "concat", + group: "popularExpressions", + }), + ]); + + expect( + suggest_({ + source: "", + ...expressionOpts, + startRule: "aggregation", + query: createQuery({ + metadata, + query: DEFAULT_QUERY, + }), + stageIndex: -1, + metadata, + getColumnIcon: () => "icon", + }).suggestions, + ).toEqual([ + expect.objectContaining({ + name: "Count", + group: "popularAggregations", + }), + expect.objectContaining({ + name: "Distinct", + group: "popularAggregations", + }), + expect.objectContaining({ + name: "CountIf", + group: "popularAggregations", + }), + expect.objectContaining({ + name: "Sum", + group: "popularAggregations", + }), + expect.objectContaining({ + name: "Average", + group: "popularAggregations", + }), + ]); + }); }); }); diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.styled.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.styled.tsx index 457ba221eb4c915c72eb13e6a349e72d987cced5..9c46c48bf359f77e7bc3fbea6e87d8bb3816005c 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.styled.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.styled.tsx @@ -7,11 +7,10 @@ import { PopoverHoverTarget as BasePopoverHoverTarget, } from "metabase/components/MetadataInfo/InfoIcon"; import { color } from "metabase/lib/colors"; +import { Icon } from "metabase/ui"; export const ExpressionList = styled.ul` - min-width: 150px; - max-height: 350px; - overflow-y: auto; + min-width: 250px; `; export const SuggestionMatch = styled.span` @@ -29,7 +28,32 @@ export const ExpressionListItem = styled.li<{ isHighlighted: boolean }>` padding: 0 0.875rem; padding-right: 0.5rem; cursor: pointer; - min-height: 1.625rem; + height: 2rem; + display: flex; + align-items: center; + + &:hover { + ${highlighted} + } + + ${props => props.isHighlighted && highlighted} +`; + +export const ExpressionListFooter = styled.a<{ isHighlighted: boolean }>` + border-top: 1px solid ${color("border")}; + background: white; + height: 2rem; + font-weight: bold; + color: ${color("text-medium")}; + + position: sticky; + bottom: 0; + + display: flex; + align-items: center; + justify-content: space-between; + + padding-left: 0.875rem; &:hover { ${highlighted} @@ -38,6 +62,11 @@ export const ExpressionListItem = styled.li<{ isHighlighted: boolean }>` ${props => props.isHighlighted && highlighted} `; +export const ExternalIcon = styled(Icon)` + height: 0.8rem; + margin-right: 0.5rem; +`; + export const SuggestionTitle = styled.span` margin-right: 1.5em; `; @@ -58,3 +87,17 @@ export const PopoverHoverTarget = styled(BasePopoverHoverTarget)` visibility: visible; } `; + +export const GroupTitle = styled(ExpressionListItem)` + text-transform: uppercase; + font-weight: bold; + font-size: 12px; + color: ${color("text-medium")}; + pointer-events: none; + + border-top: 1px solid ${color("border")}; + + &:first-child { + border-top: none; + } +`; diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.tsx index 67dd19fe1e1124f15b4a429dea95c06c03495438..bd77b0d8639107f9c68af1cf67f0b1bf72c8e11c 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.tsx @@ -1,5 +1,11 @@ import cx from "classnames"; -import { useEffect, useRef, useCallback, type ReactNode } from "react"; +import { + useEffect, + useRef, + useCallback, + type ReactNode, + type MouseEvent, +} from "react"; import { t } from "ttag"; import _ from "underscore"; @@ -10,68 +16,168 @@ import { color } from "metabase/lib/colors"; import { isObscured } from "metabase/lib/dom"; import { DelayGroup, Icon, type IconName, Popover } from "metabase/ui"; import type * as Lib from "metabase-lib"; -import type { Suggestion } from "metabase-lib/v1/expressions/suggest"; +import type { + Suggestion, + GroupName, +} from "metabase-lib/v1/expressions/suggest"; +import { GROUPS } from "metabase-lib/v1/expressions/suggest"; import { ExpressionEditorHelpTextContent } from "../ExpressionEditorHelpText"; +import type { SuggestionFooter } from "../ExpressionEditorTextfield"; import { ExpressionListItem, + ExpressionListFooter, + ExternalIcon, ExpressionList, SuggestionMatch, SuggestionTitle, + GroupTitle, QueryColumnInfoIcon, PopoverHoverTarget, } from "./ExpressionEditorSuggestions.styled"; +type WithIndex<T> = T & { + index: number; +}; + export function ExpressionEditorSuggestions({ query, stageIndex, suggestions = [], onSuggestionMouseDown, + open, highlightedIndex, children, }: { query: Lib.Query; stageIndex: number; - suggestions?: Suggestion[]; + suggestions?: (Suggestion | SuggestionFooter)[]; onSuggestionMouseDown: (index: number) => void; + open: boolean; highlightedIndex: number; children: ReactNode; }) { + const ref = useRef(null); + + const withIndex = suggestions.map((suggestion, index) => ({ + ...suggestion, + index, + })); + + const items = withIndex.filter( + (suggestion): suggestion is WithIndex<Suggestion> => + !("footer" in suggestion), + ); + + const footers = withIndex.filter( + (suggestion): suggestion is WithIndex<SuggestionFooter> => + "footer" in suggestion, + ); + + const groups = group(items); + + function handleMouseDown(evt: MouseEvent) { + if (evt.target === ref.current) { + evt.preventDefault(); + evt.stopPropagation(); + } + } + return ( <Popover position="bottom-start" - opened={suggestions?.length > 0} + opened={open && suggestions.length > 0} radius="xs" withinPortal zIndex={300} - returnFocus > <Popover.Target>{children}</Popover.Target> <Popover.Dropdown> <DelayGroup> <ExpressionList data-testid="expression-suggestions-list" - className={CS.pb1} + ref={ref} + onMouseDownCapture={handleMouseDown} > - {suggestions.map((suggestion: Suggestion, idx: number) => ( - <ExpressionEditorSuggestionsListItem - key={`suggesion-${idx}`} - query={query} - stageIndex={stageIndex} - suggestion={suggestion} - isHighlighted={idx === highlightedIndex} - index={idx} - onMouseDown={onSuggestionMouseDown} - /> - ))} + <ExpressionEditorSuggestionsListGroup + suggestions={groups._none} + query={query} + stageIndex={stageIndex} + highlightedIndex={highlightedIndex} + onSuggestionMouseDown={onSuggestionMouseDown} + /> + <ExpressionEditorSuggestionsListGroup + name="popularAggregations" + suggestions={groups.popularAggregations} + query={query} + stageIndex={stageIndex} + highlightedIndex={highlightedIndex} + onSuggestionMouseDown={onSuggestionMouseDown} + /> + <ExpressionEditorSuggestionsListGroup + name="popularExpressions" + suggestions={groups.popularExpressions} + query={query} + stageIndex={stageIndex} + highlightedIndex={highlightedIndex} + onSuggestionMouseDown={onSuggestionMouseDown} + /> </ExpressionList> + {footers.map(suggestion => ( + <Footer + key={suggestion.index} + suggestion={suggestion} + highlightedIndex={highlightedIndex} + /> + ))} </DelayGroup> </Popover.Dropdown> </Popover> ); } +function ExpressionEditorSuggestionsListGroup({ + name, + query, + stageIndex, + suggestions = [], + onSuggestionMouseDown, + highlightedIndex, +}: { + name?: GroupName; + query: Lib.Query; + stageIndex: number; + suggestions?: Suggestion[]; + onSuggestionMouseDown: (index: number) => void; + highlightedIndex: number; +}) { + const definition = name && GROUPS[name]; + + if (suggestions.length === 0) { + return null; + } + + return ( + <> + {definition?.displayName && ( + <GroupTitle isHighlighted={false}>{definition.displayName}</GroupTitle> + )} + {suggestions.map((suggestion: SuggestionWithIndex) => ( + <ExpressionEditorSuggestionsListItem + key={`suggestion-${suggestion.index}`} + query={query} + stageIndex={stageIndex} + suggestion={suggestion} + isHighlighted={suggestion.index === highlightedIndex} + index={suggestion.index} + onMouseDown={onSuggestionMouseDown} + /> + ))} + </> + ); +} + function ExpressionEditorSuggestionsListItem({ query, stageIndex, @@ -87,8 +193,8 @@ function ExpressionEditorSuggestionsListItem({ onMouseDown: (index: number) => void; suggestion: Suggestion; }) { - const { icon, helpText, name, range = [] } = suggestion; - const [start = 0, end = name.length - 1] = range; + const { icon, helpText, range = [] } = suggestion; + const [start = 0, end = 0] = range; const { normal, highlighted } = colorForIcon(icon); @@ -157,6 +263,30 @@ function ExpressionEditorSuggestionsListItem({ ); } +function Footer({ + suggestion, + highlightedIndex, +}: { + suggestion: WithIndex<SuggestionFooter>; + highlightedIndex: number; +}) { + function handleMouseDownCapture(evt: MouseEvent) { + // prevent the dropdown from closing + evt.preventDefault(); + } + + return ( + <ExpressionListFooter + target="_blank" + href={suggestion.href} + onMouseDownCapture={handleMouseDownCapture} + isHighlighted={highlightedIndex === suggestion.index} + > + {suggestion.name} <ExternalIcon name={suggestion.icon} /> + </ExpressionListFooter> + ); +} + function colorForIcon(icon: string | undefined | null) { switch (icon) { case "segment": @@ -172,3 +302,29 @@ function colorForIcon(icon: string | undefined | null) { }; } } + +type SuggestionWithIndex = Suggestion & { + index: number; +}; + +type Groups = { + [key in GroupName | "_none"]: SuggestionWithIndex[]; +}; + +function group(suggestions: Suggestion[]): Groups { + const groups: Groups = { + _none: [], + popularAggregations: [], + popularExpressions: [], + }; + + suggestions.forEach(function (suggestion) { + if (suggestion.group) { + groups[suggestion.group].push(suggestion); + } else { + groups._none.push(suggestion); + } + }); + + return groups; +} diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.unit.spec.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.unit.spec.tsx index 4342eb1abf4bc379931de4142f1d00182ac0b87d..8f702df1b61590b2932724f8b735ac55636b9186 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorSuggestions/ExpressionEditorSuggestions.unit.spec.tsx @@ -19,25 +19,27 @@ type WrapperProps = { suggestions?: Suggestion[]; highlightedIndex: number; onSuggestionMouseDown: () => void; + startRule: string; }; function Wrapper(props: WrapperProps) { return ( - <ExpressionEditorSuggestions {...props}> + <ExpressionEditorSuggestions {...props} open> <div>target</div> </ExpressionEditorSuggestions> ); } type SetupOpts = { - source: string; + source?: string; + startRule: string; }; -function setup(opts: SetupOpts) { +function setup({ source = "", startRule }: SetupOpts) { const query = createQuery({ metadata: METADATA }); const stageIndex = 0; const { suggestions } = suggest({ - source: opts.source, + source, query, stageIndex, metadata: METADATA, @@ -51,6 +53,7 @@ function setup(opts: SetupOpts) { suggestions, onSuggestionMouseDown: jest.fn(), highlightedIndex: -1, + startRule, }; const { rerender } = renderWithProviders(<Wrapper {...props} />); @@ -60,8 +63,8 @@ function setup(opts: SetupOpts) { } describe("ExpressionEditorSuggestions", () => { - test("suggestions items should show column info icon", async () => { - setup({ source: "[" }); + it("should render with the column info icon", async () => { + setup({ source: "[", startRule: "expression" }); await waitFor(() => screen.findAllByTestId("expression-suggestions-list-item"), @@ -73,7 +76,7 @@ describe("ExpressionEditorSuggestions", () => { }); test("suggestions items should show function helptext info icons", async () => { - setup({ source: "con" }); + setup({ source: "con", startRule: "expression" }); await waitFor(() => screen.findAllByTestId("expression-suggestions-list-item"), @@ -83,4 +86,20 @@ describe("ExpressionEditorSuggestions", () => { 1, ); }); + + it("should show functions when first opened", () => { + setup({ startRule: "expression" }); + expect(screen.getByText("Most used functions")).toBeInTheDocument(); + + expect(screen.getByText("case")).toBeInTheDocument(); + expect(screen.getByText("coalesce")).toBeInTheDocument(); + }); + + it("should not include popular functions when text has been typed", () => { + setup({ source: "[", startRule: "expression" }); + expect(screen.queryByText("Most used functions")).not.toBeInTheDocument(); + + expect(screen.queryByText("case")).not.toBeInTheDocument(); + expect(screen.queryByText("coalesce")).not.toBeInTheDocument(); + }); }); diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.tsx index 964a516a59b9d662de4032f761543fb81c26ecc0..0884f182d2db8c209385adb34827fcfc80ac0162 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.tsx @@ -11,12 +11,17 @@ import _ from "underscore"; import { getColumnIcon } from "metabase/common/utils/columns"; import ExplicitSize from "metabase/components/ExplicitSize"; import { getMetadata } from "metabase/selectors/metadata"; +import { getShowMetabaseLinks } from "metabase/selectors/whitelabel"; +import type { IconName } from "metabase/ui"; import * as Lib from "metabase-lib"; import { isExpression } from "metabase-lib/v1/expressions"; import { diagnose } from "metabase-lib/v1/expressions/diagnostics"; import { format } from "metabase-lib/v1/expressions/format"; import { processSource } from "metabase-lib/v1/expressions/process"; -import type { Suggestion } from "metabase-lib/v1/expressions/suggest"; +import type { + SuggestArgs, + Suggestion, +} from "metabase-lib/v1/expressions/suggest"; import { suggest } from "metabase-lib/v1/expressions/suggest"; import { tokenize } from "metabase-lib/v1/expressions/tokenizer"; import type { @@ -40,6 +45,49 @@ import { ace.config.set("basePath", "/assets/ui/"); ace.config.set("useStrictCSP", true); +export type SuggestionFooter = { + footer: true; + name: string; + icon: IconName; + href: string; +}; + +type SuggestWithFooters = { + suggestions: (Suggestion | SuggestionFooter)[]; + helpText?: HelpText; +}; + +export function suggestWithFooters( + args: SuggestArgs & { showMetabaseLinks: boolean }, +): SuggestWithFooters { + const res = suggest(args); + + const suggestions: (Suggestion | SuggestionFooter)[] = res.suggestions ?? []; + + if (args.showMetabaseLinks && args.source === "") { + if (args.startRule === "aggregation") { + suggestions.push({ + footer: true, + name: t`View all aggregations`, + icon: "external", + href: "https://www.metabase.com/docs/latest/questions/query-builder/expressions-list#aggregations", + }); + } else { + suggestions.push({ + footer: true, + name: t`View all functions`, + icon: "external", + href: "https://www.metabase.com/docs/latest/questions/query-builder/expressions-list#functions", + }); + } + } + + return { + ...res, + suggestions, + }; +} + const ACE_OPTIONS = { behavioursEnabled: false, indentedSoftWrap: false, @@ -75,11 +123,12 @@ interface ExpressionEditorTextfieldProps { expressionClause: Lib.ExpressionClause | null, ) => void; helpTextTarget: RefObject<HTMLElement>; + showMetabaseLinks: boolean; } interface ExpressionEditorTextfieldState { source: string; - suggestions: Suggestion[]; + suggestions: (Suggestion | SuggestionFooter)[]; highlightedSuggestionIndex: number; isFocused: boolean; errorMessage: ErrorWithMessage | null; @@ -97,6 +146,10 @@ function transformPropsToState( clause, query, stageIndex, + expressionPosition, + metadata, + reportTimezone, + showMetabaseLinks, } = props; const expressionFromClause = clause ? Lib.legacyExpressionForExpressionClause(query, stageIndex, clause) @@ -108,11 +161,24 @@ function transformPropsToState( query, }); + const { suggestions = [], helpText = null } = suggestWithFooters({ + reportTimezone, + startRule, + source, + targetOffset: 0, + expressionPosition, + query, + stageIndex, + metadata, + getColumnIcon, + showMetabaseLinks, + }); + return { source, highlightedSuggestionIndex: 0, - helpText: null, - suggestions: [], + helpText, + suggestions, isFocused: false, errorMessage: null, hasChanges: false, @@ -121,6 +187,7 @@ function transformPropsToState( const mapStateToProps = (state: State) => ({ metadata: getMetadata(state), + showMetabaseLinks: getShowMetabaseLinks(state), }); const CURSOR_DEBOUNCE_INTERVAL = 10; @@ -181,12 +248,34 @@ class ExpressionEditorTextfield extends React.Component< } } + handleKeypress = (evt: KeyboardEvent) => { + if (evt.key !== "Enter") { + return; + } + + evt.preventDefault(); + evt.stopPropagation(); + this.handleEnter(); + }; + + textarea() { + return this.input.current?.refEditor?.getElementsByTagName("textarea")[0]; + } + componentDidMount() { if (this.input.current) { const { editor } = this.input.current; // "ExpressionMode" constructor is not typed, so cast it here explicitly const mode = new ExpressionMode() as unknown as Ace.SyntaxMode; + // HACK: manually register the keypress event for the enter key, + // since ACE does not seem to call the event handlers in time for + // them to do certain things, like window.open. + // + // Without this hack, popups get blocked since they are not + // considered by the browser to be in response to a user action. + this.textarea()?.addEventListener("keypress", this.handleKeypress); + editor.getSession().setMode(mode); editor.setOptions({ @@ -216,10 +305,20 @@ class ExpressionEditorTextfield extends React.Component< } } + componentWillUnmount() { + this.textarea()?.removeEventListener("keypress", this.handleKeypress); + } + onSuggestionSelected = (index: number) => { const { source, suggestions } = this.state; const suggestion = suggestions && suggestions[index]; + if ("footer" in suggestion) { + // open link in new window + window.open(suggestion.href, "_blank"); + return; + } + if (this.input.current && suggestion) { const { editor } = this.input.current; const { tokens } = tokenize(source); @@ -248,9 +347,10 @@ class ExpressionEditorTextfield extends React.Component< // clicking on the autocomplete setTimeout(() => editor.moveCursorTo(row, caretPos)); } else { - const newExpression = source + suggestion.text; - this.handleExpressionChange(newExpression); - editor.moveCursorTo(row, newExpression.length); + const updatedExpression = source + suggestion.text; + this.handleExpressionChange(updatedExpression); + const caretPos = updatedExpression.length; + setTimeout(() => editor.moveCursorTo(row, caretPos)); } } }; @@ -361,7 +461,9 @@ class ExpressionEditorTextfield extends React.Component< this.updateSuggestions([]); } - updateSuggestions(suggestions: Suggestion[] | undefined = []) { + updateSuggestions( + suggestions: (Suggestion | SuggestionFooter)[] | undefined = [], + ) { this.setState({ suggestions }); // Correctly bind Tab depending on whether suggestions are available or not @@ -488,9 +590,10 @@ class ExpressionEditorTextfield extends React.Component< metadata, expressionPosition, startRule = ExpressionEditorTextfield.defaultProps.startRule, + showMetabaseLinks, } = this.props; const { source } = this.state; - const { suggestions, helpText } = suggest({ + const { suggestions, helpText } = suggestWithFooters({ reportTimezone, startRule, source, @@ -500,6 +603,7 @@ class ExpressionEditorTextfield extends React.Component< stageIndex, metadata, getColumnIcon, + showMetabaseLinks, }); this.setState({ helpText: helpText || null }); @@ -529,6 +633,7 @@ class ExpressionEditorTextfield extends React.Component< } commands: ICommand[] = [ + // Note: Enter is handled manually (see componentDidMount) { name: "arrowDown", bindKey: { win: "Down", mac: "Down" }, @@ -543,13 +648,6 @@ class ExpressionEditorTextfield extends React.Component< this.handleArrowUp(); }, }, - { - name: "enter", - bindKey: { win: "Enter", mac: "Enter" }, - exec: () => { - this.handleEnter(); - }, - }, { name: "chooseSuggestion", // @ts-expect-error Based on typings null is not a valid value, but bindKey is assigned dynamically if there are suggestions available. @@ -587,6 +685,7 @@ class ExpressionEditorTextfield extends React.Component< suggestions={suggestions} onSuggestionMouseDown={this.onSuggestionSelected} highlightedIndex={highlightedSuggestionIndex} + open={isFocused} > <EditorContainer isFocused={isFocused} diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.unit.spec.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..038ffefc450d2461c1156fceaf9d80f827c7a450 --- /dev/null +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/ExpressionEditorTextfield.unit.spec.tsx @@ -0,0 +1,63 @@ +import { createMockMetadata } from "__support__/metadata"; +import { getColumnIcon } from "metabase/common/utils/columns"; +import { createQuery } from "metabase-lib/test-helpers"; +import { createSampleDatabase } from "metabase-types/api/mocks/presets"; + +import { suggestWithFooters } from "./ExpressionEditorTextfield"; + +const METADATA = createMockMetadata({ + databases: [createSampleDatabase()], +}); + +type SetupOpts = { + startRule: string; +}; + +function setup({ startRule }: SetupOpts) { + const query = createQuery({ metadata: METADATA }); + const stageIndex = 0; + const { suggestions } = suggestWithFooters({ + source: "", + query, + stageIndex, + metadata: METADATA, + startRule, + getColumnIcon, + showMetabaseLinks: true, + }); + + return suggestions; +} + +describe("suggestWithFooters", () => { + it("should return correct functions link for expressions", () => { + const suggestions = setup({ startRule: "expression" }); + + expect(suggestions.find(suggestion => "footer" in suggestion)).toEqual({ + footer: true, + name: "View all functions", + icon: "external", + href: "https://www.metabase.com/docs/latest/questions/query-builder/expressions-list#functions", + }); + }); + + it("should return correct functions link for filters", () => { + const suggestions = setup({ startRule: "boolean" }); + expect(suggestions.find(suggestion => "footer" in suggestion)).toEqual({ + footer: true, + name: "View all functions", + icon: "external", + href: "https://www.metabase.com/docs/latest/questions/query-builder/expressions-list#functions", + }); + }); + + it("should return correct functions link for aggregations", () => { + const suggestions = setup({ startRule: "aggregation" }); + expect(suggestions.find(suggestion => "footer" in suggestion)).toEqual({ + footer: true, + name: "View all aggregations", + icon: "external", + href: "https://www.metabase.com/docs/latest/questions/query-builder/expressions-list#aggregations", + }); + }); +}); diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/index.ts b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/index.ts index e729afaee9b51e758a2e9977b372c25c51c4e241..011fce73533121b120e484b3aca6e0f13e04b297 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/index.ts +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield/index.ts @@ -1 +1,4 @@ -export { default as ExpressionEditorTextfield } from "./ExpressionEditorTextfield"; +export { + default as ExpressionEditorTextfield, + type SuggestionFooter, +} from "./ExpressionEditorTextfield"; diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.tsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.tsx index e3e44e1e93a12932caac2a0a87d5c6b71f8f9ef7..fbe1d7466a0846e4a8f9caa9ab247c69c4c19558 100644 --- a/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.tsx +++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionWidget.tsx @@ -118,7 +118,7 @@ export const ExpressionWidget = <Clause extends object = Lib.ExpressionClause>( }; return ( - <Container> + <Container data-testid="expression-editor"> {header} <ExpressionFieldWrapper> <FieldLabel htmlFor="expression-content">