Skip to content
Snippets Groups Projects
Unverified Commit 6a927dee authored by Phoomparin Mano's avatar Phoomparin Mano Committed by GitHub
Browse files

fix(sdk): support hiding columns in InteractiveQuestion (#49013)

* investigate hide columns

* add update viz settings in the sdk

* cleanup implementation

* add storybook for interactive question

* use the question callback to update viz settings

* remove onUpdateVisualizationSettings

* revert change to onUpdateVisualizationSettings

* add click action behaviour

* fix misspelling

* add onUpdateQuestion mock function

* add unit test for click action behaviour

* add e2e test for hiding question column header
parent f022ed81
No related branches found
No related tags found
No related merge requests found
Showing
with 111 additions and 14 deletions
......@@ -5,6 +5,8 @@ import {
popover,
restore,
setTokenFeatures,
tableHeaderClick,
tableInteractive,
visitFullAppEmbeddingUrl,
} from "e2e/support/helpers";
import {
......@@ -100,6 +102,22 @@ describeSDK("scenarios > embedding-sdk > interactive-question", () => {
cy.icon("warning").should("not.exist");
});
it("should be able to hide columns from a table", () => {
cy.wait("@cardQuery").then(({ response }) => {
expect(response?.statusCode).to.equal(202);
});
tableInteractive().findByText("Max of Quantity").should("be.visible");
tableHeaderClick("Max of Quantity");
popover()
.findByTestId("click-actions-sort-control-formatting-hide")
.click();
tableInteractive().findByText("Max of Quantity").should("not.exist");
});
it("can save a question", () => {
cy.intercept("POST", "/api/card").as("createCard");
......
......@@ -7,6 +7,7 @@ import {
} from "embedding-sdk/components/private/PublicComponentWrapper";
import CS from "metabase/css/core/index.css";
import QueryVisualization from "metabase/query_builder/components/QueryVisualization";
import type Question from "metabase-lib/v1/Question";
import { useInteractiveQuestionContext } from "../context";
......@@ -19,6 +20,7 @@ export const QuestionVisualization = () => {
isQueryRunning,
navigateToNewCard,
onNavigateBack,
updateQuestion,
} = useInteractiveQuestionContext();
if (isQuestionLoading) {
......@@ -46,6 +48,9 @@ export const QuestionVisualization = () => {
mode={mode}
navigateToNewCardInsideQB={navigateToNewCard}
onNavigateBack={onNavigateBack}
onUpdateQuestion={(question: Question) =>
updateQuestion(question, { run: false })
}
/>
);
};
......@@ -139,6 +139,7 @@ export default class VisualizationResult extends Component {
onSelectTimelineEvents={this.props.selectTimelineEvents}
onDeselectTimelineEvents={this.props.deselectTimelineEvents}
onOpenChartSettings={this.props.onOpenChartSettings}
onUpdateQuestion={this.props.onUpdateQuestion}
onUpdateWarnings={this.props.onUpdateWarnings}
onUpdateVisualizationSettings={
this.props.onUpdateVisualizationSettings
......
......@@ -54,6 +54,7 @@ export const ViewMainContainer = props => {
noHeader
className={CS.spread}
mode={queryMode}
onUpdateQuestion={updateQuestion}
/>
</StyledDebouncedFrame>
<TimeseriesChrome
......
import { t } from "ttag";
import { onUpdateVisualizationSettings } from "metabase/query_builder/actions";
import type { LegacyDrill } from "metabase/visualizations/types";
import * as Lib from "metabase-lib";
import { findColumnSettingIndexesForColumns } from "metabase-lib/v1/queries/utils/dataset";
......@@ -33,7 +32,7 @@ export const HideColumnAction: LegacyDrill = ({
icon: "eye_crossed_out",
tooltip: t`Hide column`,
default: true,
action: () => {
question: () => {
const columnSettings = settings?.["table.columns"] ?? [];
const [columnSettingIndex] = findColumnSettingIndexesForColumns(
[column],
......@@ -48,10 +47,9 @@ export const HideColumnAction: LegacyDrill = ({
};
}
return onUpdateVisualizationSettings({
"table.columns": columnSettingsCopy,
});
return question.updateSettings({ "table.columns": columnSettingsCopy });
},
questionChangeBehavior: "updateQuestion",
},
];
};
......@@ -86,7 +86,11 @@ export const ClickActionControl = ({
case "sort":
return (
<Tooltip tooltip={action.tooltip}>
<SortControl onlyIcon onClick={handleClick}>
<SortControl
onlyIcon
onClick={handleClick}
data-testid={`click-actions-sort-control-${action.name}`}
>
{typeof action.icon === "string" && (
<Icon size={14} name={action.icon as unknown as IconName} />
)}
......
......@@ -11,6 +11,7 @@ import type {
RegularClickAction,
} from "metabase/visualizations/types";
import { isPopoverClickAction } from "metabase/visualizations/types";
import type Question from "metabase-lib/v1/Question";
import type { Series } from "metabase-types/api";
import type { Dispatch } from "metabase-types/store";
......@@ -24,6 +25,7 @@ interface ChartClickActionsProps {
dispatch: Dispatch;
onChangeCardAndRun: OnChangeCardAndRun;
onUpdateVisualizationSettings: () => void;
onUpdateQuestion?: (question: Question) => void;
onClose?: () => void;
}
......@@ -49,13 +51,14 @@ export class ClickActionsPopover extends Component<
};
handleClickAction = (action: RegularClickAction) => {
const { dispatch, onChangeCardAndRun } = this.props;
const { dispatch, onChangeCardAndRun, onUpdateQuestion } = this.props;
if (isPopoverClickAction(action)) {
this.setState({ popoverAction: action });
} else {
const didPerform = performAction(action, {
dispatch,
onChangeCardAndRun,
onUpdateQuestion,
});
if (didPerform) {
this.close();
......
......@@ -56,6 +56,7 @@ const defaultProps = {
isSettings: false,
isQueryBuilder: false,
isEmbeddingSdk: false,
onUpdateQuestion: () => {},
onUpdateVisualizationSettings: () => {},
// prefer passing in a function that doesn't cause the application to reload
onChangeLocation: location => {
......@@ -540,6 +541,7 @@ class Visualization extends PureComponent {
clicked={clicked}
clickActions={regularClickActions}
onChangeCardAndRun={this.handleOnChangeCardAndRun}
onUpdateQuestion={this.props.onUpdateQuestion}
onClose={this.hideActions}
series={series}
onUpdateVisualizationSettings={onUpdateVisualizationSettings}
......
......@@ -4,7 +4,10 @@ import _ from "underscore";
import { setParameterValuesFromQueryParams } from "metabase/dashboard/actions";
import { open } from "metabase/lib/dom";
export function performAction(action, { dispatch, onChangeCardAndRun }) {
export function performAction(
action,
{ dispatch, onChangeCardAndRun, onUpdateQuestion },
) {
let didPerform = false;
if (action.action) {
const reduxAction = action.action();
......@@ -28,14 +31,22 @@ export function performAction(action, { dispatch, onChangeCardAndRun }) {
}
}
if (action.question) {
const { questionChangeBehavior = "changeCardAndRun" } = action;
const question = action.question();
const extra = action?.extra?.() ?? {};
if (question) {
onChangeCardAndRun({
nextCard: question.card(),
...extra,
objectId: extra.objectId,
});
if (questionChangeBehavior === "changeCardAndRun") {
onChangeCardAndRun({
nextCard: question.card(),
...extra,
objectId: extra.objectId,
});
} else if (questionChangeBehavior === "updateQuestion") {
onUpdateQuestion(question);
}
didPerform = true;
}
}
......
import MetabaseSettings from "metabase/lib/settings";
import type { UrlClickAction } from "metabase/visualizations/types";
import type {
QuestionChangeClickAction,
UrlClickAction,
} from "metabase/visualizations/types";
import Question from "metabase-lib/v1/Question";
import { performAction } from "./action";
......@@ -16,6 +20,7 @@ describe("performAction", () => {
const extraProps = {
dispatch: jest.fn(),
onChangeCardAndRun: jest.fn(),
onUpdateQuestion: jest.fn(),
};
expect(performAction(action, extraProps)).toBe(true);
......@@ -79,6 +84,7 @@ describe("performAction", () => {
const extraProps = {
dispatch: jest.fn(),
onChangeCardAndRun: jest.fn(),
onUpdateQuestion: jest.fn(),
};
expect(performAction(action, extraProps)).toBe(true);
......@@ -117,6 +123,7 @@ describe("performAction", () => {
const extraProps = {
dispatch: jest.fn(),
onChangeCardAndRun: jest.fn(),
onUpdateQuestion: jest.fn(),
};
expect(performAction(action, extraProps)).toBe(true);
......@@ -156,6 +163,7 @@ describe("performAction", () => {
const extraProps = {
dispatch: jest.fn(),
onChangeCardAndRun: jest.fn(),
onUpdateQuestion: jest.fn(),
};
expect(performAction(action, extraProps)).toBe(true);
......@@ -194,6 +202,7 @@ describe("performAction", () => {
const extraProps = {
dispatch: jest.fn(),
onChangeCardAndRun: jest.fn(),
onUpdateQuestion: jest.fn(),
};
expect(performAction(action, extraProps)).toBe(true);
......@@ -218,4 +227,38 @@ describe("performAction", () => {
});
});
});
it("performs question change actions according to question change behavior", () => {
const mockQuestion = Question.create();
const defaultQuestionAction: QuestionChangeClickAction = {
name: "bar",
question: () => mockQuestion,
buttonType: "horizontal",
section: "auto",
};
const updateQuestionAction: QuestionChangeClickAction = {
...defaultQuestionAction,
questionChangeBehavior: "updateQuestion",
};
const extraProps = {
dispatch: jest.fn(),
onChangeCardAndRun: jest.fn(),
onUpdateQuestion: jest.fn(),
};
expect(performAction(defaultQuestionAction, extraProps)).toBe(true);
expect(extraProps.onChangeCardAndRun).toHaveBeenCalledTimes(1);
expect(extraProps.onChangeCardAndRun).toHaveBeenCalledWith({
nextCard: mockQuestion.card(),
});
expect(performAction(updateQuestionAction, extraProps)).toBe(true);
expect(extraProps.onUpdateQuestion).toHaveBeenCalledTimes(1);
expect(extraProps.onUpdateQuestion).toHaveBeenCalledWith(mockQuestion);
});
});
......@@ -66,8 +66,19 @@ type ReduxClickActionBase = {
export type ReduxClickAction = ClickActionBase & ReduxClickActionBase;
/**
* What should happen when a "question change" click action is performed?
*
* - `changeCardAndRun`: the card is changed and the query is run. this is the default behavior.
* - `updateQuestion`: the question is updated (without running the query)
*/
export type QuestionChangeClickActionBehavior =
| "changeCardAndRun"
| "updateQuestion";
export type QuestionChangeClickActionBase = {
question: () => Question;
questionChangeBehavior?: QuestionChangeClickActionBehavior;
};
export type QuestionChangeClickAction = ClickActionBase &
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment