From dc59d0851daf05fead9c7331e83d7fcc3fa328e5 Mon Sep 17 00:00:00 2001 From: Dalton <daltojohnso@users.noreply.github.com> Date: Wed, 22 Dec 2021 09:51:25 -0800 Subject: [PATCH] Add DimensionInfoPopover to FieldsPicker (#19286) * Add DimensionInfoPopover to FieldsPicker * add e2e tests * skip e2e tests for now --- .../notebook/steps/FieldsPicker.jsx | 26 +++--- .../scenarios/question/notebook.cy.spec.js | 84 +++++++++++++------ 2 files changed, 74 insertions(+), 36 deletions(-) diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx b/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx index 206c9144a65..6490db35590 100644 --- a/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx +++ b/frontend/src/metabase/query_builder/components/notebook/steps/FieldsPicker.jsx @@ -4,6 +4,7 @@ import React from "react"; import { t } from "ttag"; import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; +import DimensionInfoPopover from "metabase/components/MetadataInfo/DimensionInfoPopover"; import CheckBox from "metabase/components/CheckBox"; import StackedCheckBox from "metabase/components/StackedCheckBox"; @@ -48,18 +49,19 @@ export default function FieldsPicker({ </li> )} {dimensions.map(dimension => ( - <li key={dimension.key()} className="px1 pb1 flex align-center"> - <CheckBox - data-testid={`field-${dimension.displayName()}`} - disabled={disableSelected && selected.has(dimension.key())} - checked={selected.has(dimension.key())} - label={dimension.displayName()} - onChange={() => { - onToggleDimension(dimension, !selected.has(dimension.key())); - }} - className="mr1" - /> - </li> + <DimensionInfoPopover dimension={dimension} key={dimension.key()}> + <li className="px1 pb1 flex align-center"> + <CheckBox + disabled={disableSelected && selected.has(dimension.key())} + checked={selected.has(dimension.key())} + label={dimension.displayName()} + onChange={() => { + onToggleDimension(dimension, !selected.has(dimension.key())); + }} + className="mr1" + /> + </li> + </DimensionInfoPopover> ))} </ul> </PopoverWithTrigger> diff --git a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js index 0eff1c51914..cc29c3015d2 100644 --- a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js @@ -876,6 +876,66 @@ describe("scenarios > question > notebook", () => { }); }); }); + + // intentional simplification of "Select none" to quickly + // fix users' pain caused by the inability to unselect all columns + it("select no columns select the first one", () => { + cy.visit("/question/new"); + cy.contains("Custom question").click(); + cy.contains("Sample Dataset").click(); + cy.contains("Orders").click(); + cy.findByTestId("fields-picker").click(); + + popover().within(() => { + cy.findByText("Select none").click(); + cy.findByLabelText("ID").should("be.disabled"); + cy.findByText("Tax").click(); + cy.findByLabelText("ID") + .should("be.enabled") + .click(); + }); + + visualize(); + + cy.findByText("Tax"); + cy.findByText("ID").should("not.exist"); + }); + + // flaky test + it.skip("should show an info popover when hovering over a field picker option for a table", () => { + cy.visit("/question/new"); + cy.contains("Custom question").click(); + cy.contains("Sample Dataset").click(); + cy.contains("Orders").click(); + + cy.findByTestId("fields-picker").click(); + + cy.findByText("Total").trigger("mouseenter"); + + popover().contains("The total billed amount."); + popover().contains("80.36"); + }); + + // flaky test + it.skip("should show an info popover when hovering over a field picker option for a saved question", () => { + cy.createNativeQuestion({ + name: "question a", + native: { query: "select 'foo' as a_column" }, + }); + + // start a custom question with question a + cy.visit("/question/new"); + cy.findByText("Custom question").click(); + cy.findByText("Saved Questions").click(); + cy.findByText("question a").click(); + + cy.findByTestId("fields-picker").click(); + + cy.findByText("A_COLUMN").trigger("mouseenter"); + + popover().contains("A_COLUMN"); + popover().contains("No description"); + }); }); // Extracted repro steps for #13000 @@ -924,30 +984,6 @@ function joinTwoSavedQuestions() { cy.url().should("contain", "/notebook"); }); }); - - // intentional simplification of "Select none" to quickly - // fix users' pain caused by the inability to unselect all columns - it("select no columns select the first one", () => { - cy.visit("/question/new"); - cy.contains("Custom question").click(); - cy.contains("Sample Dataset").click(); - cy.contains("Orders").click(); - cy.findByTestId("fields-picker").click(); - - cy.findByText("Select None").click(); - cy.findByTestId("field-ID").should("be.disabled"); - - cy.findByTestId("field-Tax").click(); - - cy.findByTestId("field-ID") - .should("be.enabled") - .click(); - - visualize(); - - cy.findByText("Tax"); - cy.findByText("ID").should("not.exist"); - }); } function addSimpleCustomColumn(name) { -- GitLab