Skip to content
Snippets Groups Projects
Unverified Commit eb9a9a90 authored by metabase-bot[bot]'s avatar metabase-bot[bot] Committed by GitHub
Browse files

:robot: backported "Optimize data step in the notebook editor" (#39513)

parent 31c4c299
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 110 additions and 69 deletions
frontend/src/metabase/common/components/FieldPicker/FieldPicker.tsx
+ 29
− 26
View file @ eb9a9a90
......@@ -12,8 +12,15 @@ interface FieldPickerProps {
stageIndex: number;
columns: Lib.ColumnMetadata[];
"data-testid"?: string;
isColumnSelected: (column: Lib.ColumnMetadata) => boolean;
onToggle: (columnIndex: number, isSelected: boolean) => void;
isColumnSelected: (
column: Lib.ColumnMetadata,
columnInfo: Lib.ColumnDisplayInfo,
) => boolean;
onToggle: (
column: Lib.ColumnMetadata,
isSelected: boolean,
columnIndex: number,
) => void;
onSelectAll: () => void;
onSelectNone: () => void;
}
......@@ -30,27 +37,21 @@ export const FieldPicker = ({
}: FieldPickerProps) => {
const items = useMemo(
() =>
columns.map(column => ({
...Lib.displayInfo(query, stageIndex, column),
column,
})),
[query, stageIndex, columns],
columns.map(column => {
const columnInfo = Lib.displayInfo(query, stageIndex, column);
return {
column,
columnInfo,
isSelected: isColumnSelected(column, columnInfo),
};
}),
[query, stageIndex, columns, isColumnSelected],
);
const isAll = useMemo(
() => columns.every(isColumnSelected),
[columns, isColumnSelected],
);
const isNone = useMemo(
() => columns.every(column => !isColumnSelected(column)),
[columns, isColumnSelected],
);
const isDisabledDeselection = useMemo(
() => columns.filter(isColumnSelected).length <= 1,
[columns, isColumnSelected],
);
const isAll = items.every(item => item.isSelected);
const isNone = items.every(item => !item.isSelected);
const isDisabledDeselection =
items.filter(item => item.isSelected).length <= 1;
const handleLabelToggle = () => {
if (isAll) {
......@@ -72,12 +73,14 @@ export const FieldPicker = ({
/>
</ToggleItem>
{items.map((item, index) => (
<ColumnItem key={item.longDisplayName}>
<ColumnItem key={index}>
<CheckBox
checked={isColumnSelected(item.column)}
label={item.displayName}
disabled={isColumnSelected(item.column) && isDisabledDeselection}
onChange={event => onToggle(index, event.target.checked)}
label={item.columnInfo.displayName}
checked={item.isSelected}
disabled={item.isSelected && isDisabledDeselection}
onChange={event =>
onToggle(item.column, event.target.checked, index)
}
/>
</ColumnItem>
))}
......
frontend/src/metabase/query_builder/components/notebook/steps/DataStep/DataStep.styled.tsx
+ 9
− 0
View file @ eb9a9a90
import styled from "@emotion/styled";
import IconButtonWrapper from "metabase/components/IconButtonWrapper";
import { color } from "metabase/lib/colors";
import { NotebookCell } from "../../NotebookCell";
export const DataStepCell = styled.div`
padding: ${NotebookCell.CONTAINER_PADDING};
`;
export const DataStepIconButton = styled(IconButtonWrapper)`
color: ${color("white")};
padding: ${NotebookCell.CONTAINER_PADDING};
opacity: 0.5;
`;
frontend/src/metabase/query_builder/components/notebook/steps/DataStep/DataStep.tsx
+ 60
− 35
View file @ eb9a9a90
......@@ -2,16 +2,15 @@ import { useMemo } from "react";
import { t } from "ttag";
import { FieldPicker } from "metabase/common/components/FieldPicker";
import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
import { DataSourceSelector } from "metabase/query_builder/components/DataSelector";
import { Icon, Popover, Tooltip } from "metabase/ui";
import * as Lib from "metabase-lib";
import type { DatabaseId, TableId } from "metabase-types/api";
import { FieldsPickerIcon, FIELDS_PICKER_STYLES } from "../../FieldsPickerIcon";
import { NotebookCell, NotebookCellItem } from "../../NotebookCell";
import type { NotebookStepUiComponentProps } from "../../types";
import { DataStepCell } from "./DataStep.styled";
import { DataStepCell, DataStepIconButton } from "./DataStep.styled";
export const DataStep = ({
query,
......@@ -55,15 +54,15 @@ export const DataStep = ({
inactive={!table}
right={
canSelectTableColumns && (
<DataFieldsPicker
<DataFieldPopover
query={query}
stageIndex={stageIndex}
updateQuery={updateQuery}
/>
)
}
containerStyle={FIELDS_PICKER_STYLES.notebookItemContainer}
rightContainerStyle={FIELDS_PICKER_STYLES.notebookRightItemContainer}
containerStyle={{ padding: 0 }}
rightContainerStyle={{ width: 37, height: 37, padding: 0 }}
data-testid="data-step-cell"
>
<DataSourceSelector
......@@ -81,36 +80,67 @@ export const DataStep = ({
);
};
interface DataFieldsPickerProps {
interface DataFieldPopoverProps {
query: Lib.Query;
stageIndex: number;
updateQuery: (query: Lib.Query) => Promise<void>;
}
export const DataFieldsPicker = ({
function DataFieldPopover({
query,
stageIndex,
updateQuery,
}: DataFieldsPickerProps) => {
}: DataFieldPopoverProps) {
return (
<Popover position="bottom-start">
<Popover.Target>
<Tooltip label={t`Pick columns`}>
<DataStepIconButton
aria-label={t`Pick columns`}
data-testid="fields-picker"
>
<Icon name="chevrondown" />
</DataStepIconButton>
</Tooltip>
</Popover.Target>
<Popover.Dropdown>
<DataFieldPicker
query={query}
stageIndex={stageIndex}
updateQuery={updateQuery}
/>
</Popover.Dropdown>
</Popover>
);
}
interface DataFieldPickerProps {
query: Lib.Query;
stageIndex: number;
updateQuery: (query: Lib.Query) => Promise<void>;
}
function DataFieldPicker({
query,
stageIndex,
updateQuery,
}: DataFieldPickerProps) {
const columns = useMemo(
() => Lib.fieldableColumns(query, stageIndex),
[query, stageIndex],
);
const handleToggle = (changedIndex: number, isSelected: boolean) => {
const nextColumns = columns.filter((_, currentIndex) => {
if (currentIndex === changedIndex) {
return isSelected;
}
const column = columns[currentIndex];
return Lib.displayInfo(query, stageIndex, column).selected;
});
const nextQuery = Lib.withFields(query, stageIndex, nextColumns);
const handleToggle = (column: Lib.ColumnMetadata, isSelected: boolean) => {
const nextQuery = isSelected
? Lib.addField(query, stageIndex, column)
: Lib.removeField(query, stageIndex, column);
updateQuery(nextQuery);
};
const checkColumnSelected = (column: Lib.ColumnMetadata) =>
!!Lib.displayInfo(query, stageIndex, column).selected;
const isColumnSelected = (column: Lib.ColumnMetadata) => {
const columnInfo = Lib.displayInfo(query, stageIndex, column);
return Boolean(columnInfo.selected);
};
const handleSelectAll = () => {
const nextQuery = Lib.withFields(query, stageIndex, []);
......@@ -123,19 +153,14 @@ export const DataFieldsPicker = ({
};
return (
<PopoverWithTrigger
triggerStyle={FIELDS_PICKER_STYLES.trigger}
triggerElement={FieldsPickerIcon}
>
<FieldPicker
query={query}
stageIndex={stageIndex}
columns={columns}
isColumnSelected={checkColumnSelected}
onToggle={handleToggle}
onSelectAll={handleSelectAll}
onSelectNone={handleSelectNone}
/>
</PopoverWithTrigger>
<FieldPicker
query={query}
stageIndex={stageIndex}
columns={columns}
isColumnSelected={isColumnSelected}
onToggle={handleToggle}
onSelectAll={handleSelectAll}
onSelectNone={handleSelectNone}
/>
);
};
}
frontend/src/metabase/query_builder/components/notebook/steps/DataStep/DataStep.unit.spec.tsx
+ 7
− 7
View file @ eb9a9a90
......@@ -134,7 +134,7 @@ describe("DataStep", () => {
await setup();
userEvent.click(screen.getByLabelText("Pick columns"));
expect(screen.getByLabelText("Select none")).toBeChecked();
expect(await screen.findByLabelText("Select none")).toBeChecked();
expect(screen.getByLabelText("ID")).toBeChecked();
expect(screen.getByLabelText("ID")).toBeEnabled();
expect(screen.getByLabelText("Tax")).toBeChecked();
......@@ -146,7 +146,7 @@ describe("DataStep", () => {
await setup(createMockNotebookStep({ query }));
userEvent.click(screen.getByLabelText("Pick columns"));
expect(screen.getByLabelText("Select all")).not.toBeChecked();
expect(await screen.findByLabelText("Select all")).not.toBeChecked();
expect(screen.getByLabelText("ID")).toBeChecked();
expect(screen.getByLabelText("ID")).toBeDisabled();
expect(screen.getByLabelText("Tax")).not.toBeChecked();
......@@ -158,7 +158,7 @@ describe("DataStep", () => {
await setup(createMockNotebookStep({ query }));
userEvent.click(screen.getByLabelText("Pick columns"));
expect(screen.getByLabelText("Select all")).not.toBeChecked();
expect(await screen.findByLabelText("Select all")).not.toBeChecked();
expect(screen.getByLabelText("ID")).toBeChecked();
expect(screen.getByLabelText("ID")).toBeEnabled();
expect(screen.getByLabelText("Tax")).not.toBeChecked();
......@@ -173,7 +173,7 @@ describe("DataStep", () => {
const { getNextColumn } = await setup(step);
userEvent.click(screen.getByLabelText("Pick columns"));
userEvent.click(screen.getByLabelText("Tax"));
userEvent.click(await screen.findByLabelText("Tax"));
expect(getNextColumn("ID").selected).toBeTruthy();
expect(getNextColumn("TAX").selected).toBeTruthy();
......@@ -184,7 +184,7 @@ describe("DataStep", () => {
const { getNextColumn } = await setup();
userEvent.click(screen.getByLabelText("Pick columns"));
userEvent.click(screen.getByLabelText("Tax"));
userEvent.click(await screen.findByLabelText("Tax"));
expect(getNextColumn("ID").selected).toBeTruthy();
expect(getNextColumn("TAX").selected).toBeFalsy();
......@@ -197,7 +197,7 @@ describe("DataStep", () => {
const { getNextColumn } = await setup(step);
userEvent.click(screen.getByLabelText("Pick columns"));
userEvent.click(screen.getByLabelText("Select all"));
userEvent.click(await screen.findByLabelText("Select all"));
expect(getNextColumn("ID").selected).toBeTruthy();
expect(getNextColumn("TAX").selected).toBeTruthy();
......@@ -208,7 +208,7 @@ describe("DataStep", () => {
const { getNextQuery } = await setup();
userEvent.click(screen.getByLabelText("Pick columns"));
userEvent.click(screen.getByLabelText("Select none"));
userEvent.click(await screen.findByLabelText("Select none"));
const nextQuery = getNextQuery();
expect(Lib.fields(nextQuery, 0)).toHaveLength(1);
......
frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinTablePicker/JoinTablePicker.tsx
+ 5
− 1
View file @ eb9a9a90
......@@ -132,7 +132,11 @@ function JoinTableColumnsPicker({
isColumnSelected,
onChange,
}: JoinTableColumnsPickerProps) {
const handleToggle = (changedIndex: number, isSelected: boolean) => {
const handleToggle = (
_column: Lib.ColumnMetadata,
isSelected: boolean,
changedIndex: number,
) => {
const nextColumns = columns.filter((_, currentIndex) =>
currentIndex === changedIndex
? isSelected
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment