From b81a929f731c69fe4005c60a9f6fc94a1450635e Mon Sep 17 00:00:00 2001 From: Dalton <daltojohnso@users.noreply.github.com> Date: Tue, 21 Dec 2021 09:02:08 -0800 Subject: [PATCH] Add DimensionInfoPopover to Table column headers (#19350) * Add DimensionInfoPopover to TableInteractive column headers * check for missing query before parsing dimension * Don't render DimensionInfoPopover until all dependent data is available * Add a min/max width to the DimensionInfoPopover content * Don't show DimensionInfoPopover when user has no metadata * Add e2e test for table column header popover * some styling tweaks * Move field value fetching back to FieldValuesList * table styling tweaks * lint fix * add loading state for field values * handle alternate field values format * Format numbers * make comment more specific * Fix e2e test * changes per review * reconcile some styling differences btwn Dimension and Table popovers Co-authored-by: Maz Ameli <maz@metabase.com> --- .../DimensionInfo/DimensionInfo.jsx | 13 +- .../DimensionInfo/DimensionInfo.styled.jsx | 17 +++ .../DimensionInfo/DimensionInfo.unit.spec.js | 4 +- .../DimensionInfoPopover.styled.tsx | 18 +++ .../DimensionInfoPopover.tsx | 18 ++- .../CategoryFingerprint.jsx | 116 ++++++++++++++++ .../CategoryFingerprint.styled.jsx | 60 +++++++++ .../CategoryFingerprint.unit.spec.js | 124 +++++++++++++++++ .../FieldFingerprintInfo.jsx | 61 ++++----- .../FieldFingerprintInfo.unit.spec.js | 36 ----- .../FieldFingerprintInfo/FieldValuesList.jsx | 44 ------ .../FieldValuesList.styled.jsx | 11 -- .../FieldValuesList.unit.spec.js | 68 ---------- .../MetadataInfo/MetadataInfo.styled.tsx | 37 ++--- .../TableInfo/TableInfo.styled.tsx | 14 ++ .../MetadataInfo/TableInfo/TableInfo.tsx | 7 +- .../components/TableInteractive.jsx | 68 +++++++--- .../scenarios/visualizations/table.cy.spec.js | 126 +++++++++++++++++- 18 files changed, 591 insertions(+), 251 deletions(-) create mode 100644 frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.styled.jsx create mode 100644 frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.styled.tsx create mode 100644 frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.jsx create mode 100644 frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.styled.jsx create mode 100644 frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.unit.spec.js delete mode 100644 frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.jsx delete mode 100644 frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.styled.jsx delete mode 100644 frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.unit.spec.js create mode 100644 frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.styled.tsx diff --git a/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.jsx b/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.jsx index 5db4bb609b2..03e8c32fdbf 100644 --- a/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.jsx +++ b/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.jsx @@ -3,21 +3,20 @@ import PropTypes from "prop-types"; import { t } from "ttag"; import Dimension from "metabase-lib/lib/Dimension"; -import DimensionLabel from "metabase/components/MetadataInfo/DimensionLabel"; -import FieldFingerprintInfo from "metabase/components/MetadataInfo/FieldFingerprintInfo"; +import { Description, EmptyDescription } from "../MetadataInfo.styled"; import { InfoContainer, - Description, - EmptyDescription, -} from "../MetadataInfo.styled"; + DimensionLabel, + FieldFingerprintInfo, +} from "./DimensionInfo.styled"; DimensionInfo.propTypes = { className: PropTypes.string, dimension: PropTypes.instanceOf(Dimension).isRequired, }; -function DimensionInfo({ className, dimension }) { +export function DimensionInfo({ className, dimension }) { const field = dimension.field(); const description = field?.description; return ( @@ -28,7 +27,7 @@ function DimensionInfo({ className, dimension }) { <EmptyDescription>{t`No description`}</EmptyDescription> )} <DimensionLabel dimension={dimension} /> - <FieldFingerprintInfo field={dimension.field()} /> + <FieldFingerprintInfo field={field} /> </InfoContainer> ); } diff --git a/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.styled.jsx b/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.styled.jsx new file mode 100644 index 00000000000..b5c0600d717 --- /dev/null +++ b/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.styled.jsx @@ -0,0 +1,17 @@ +import styled from "styled-components"; + +import _DimensionLabel from "metabase/components/MetadataInfo/DimensionLabel"; +import _FieldFingerprintInfo from "metabase/components/MetadataInfo/FieldFingerprintInfo"; +import { InfoContainer as _InfoContainer } from "metabase/components/MetadataInfo/MetadataInfo.styled"; + +export const DimensionLabel = styled(_DimensionLabel)` + font-size: 0.9em; +`; + +export const FieldFingerprintInfo = styled(_FieldFingerprintInfo)` + font-size: 0.9em; +`; + +export const InfoContainer = styled(_InfoContainer)` + gap: 0.8em; +`; diff --git a/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.unit.spec.js b/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.unit.spec.js index 1a2ddaddb28..8f48323422a 100644 --- a/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.unit.spec.js +++ b/frontend/src/metabase/components/MetadataInfo/DimensionInfo/DimensionInfo.unit.spec.js @@ -4,7 +4,7 @@ import { render, screen } from "@testing-library/react"; import { PRODUCTS, metadata } from "__support__/sample_dataset_fixture"; import Dimension from "metabase-lib/lib/Dimension"; -import DimensionInfo from "./DimensionInfo"; +import { DimensionInfo } from "./DimensionInfo"; const fieldDimension = Dimension.parseMBQL( ["field", PRODUCTS.CREATED_AT.id, null], @@ -16,7 +16,7 @@ const expressionDimension = Dimension.parseMBQL( metadata, ); -function setup(dimension) { +function setup(dimension, fieldValues) { return render(<DimensionInfo dimension={dimension} />); } diff --git a/frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.styled.tsx b/frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.styled.tsx new file mode 100644 index 00000000000..c76f2f1f137 --- /dev/null +++ b/frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.styled.tsx @@ -0,0 +1,18 @@ +import React from "react"; +import styled from "styled-components"; +import DimensionInfo from "metabase/components/MetadataInfo/DimensionInfo"; +import Dimension from "metabase-lib/lib/Dimension"; + +type DimensionInfoProps = { + dimension: Dimension; +}; + +// this makes TypeScript happy until `DimensionInfo` is typed +function _DimensionInfo(props: DimensionInfoProps) { + return <DimensionInfo {...props} />; +} + +export const WidthBoundDimensionInfo = styled(_DimensionInfo)` + width: 300px; + font-size: 14px; +`; diff --git a/frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.tsx b/frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.tsx index f54487ff830..ed1d181ac96 100644 --- a/frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.tsx +++ b/frontend/src/metabase/components/MetadataInfo/DimensionInfoPopover/DimensionInfoPopover.tsx @@ -5,9 +5,10 @@ import Dimension from "metabase-lib/lib/Dimension"; import TippyPopver, { ITippyPopoverProps, } from "metabase/components/Popover/TippyPopover"; -import DimensionInfo from "metabase/components/MetadataInfo/DimensionInfo"; import { isCypressActive } from "metabase/env"; +import { WidthBoundDimensionInfo } from "./DimensionInfoPopover.styled"; + export const POPOVER_DELAY: [number, number] = [1000, 300]; const propTypes = { @@ -21,13 +22,24 @@ type Props = { dimension: Dimension } & Pick< "children" | "placement" >; +function checkForMetadata(dimension: Dimension): boolean { + const query = dimension?.query?.(); + if (dimension && query) { + return query.metadata() != null; + } + + return false; +} + function DimensionInfoPopover({ dimension, children, placement }: Props) { - return dimension ? ( + const hasMetadata = checkForMetadata(dimension); + + return hasMetadata ? ( <TippyPopver delay={isCypressActive ? 0 : POPOVER_DELAY} interactive placement={placement || "left-start"} - content={<DimensionInfo dimension={dimension} />} + content={<WidthBoundDimensionInfo dimension={dimension} />} > {children} </TippyPopver> diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.jsx b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.jsx new file mode 100644 index 00000000000..3b37bc11923 --- /dev/null +++ b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.jsx @@ -0,0 +1,116 @@ +import React, { useEffect, useState } from "react"; +import PropTypes from "prop-types"; +import { connect } from "react-redux"; +import { t, ngettext, msgid } from "ttag"; + +import { useAsyncFunction } from "metabase/hooks/use-async-function"; +import Field from "metabase-lib/lib/metadata/Field"; +import Fields from "metabase/entities/fields"; +import { formatNumber } from "metabase/lib/formatting"; + +import { + NoWrap, + LoadingSpinner, + RelativeContainer, + Fade, + FadeAndSlide, + Container, +} from "./CategoryFingerprint.styled"; + +const propTypes = { + className: PropTypes.string, + field: PropTypes.instanceOf(Field).isRequired, + fieldValues: PropTypes.array.isRequired, + fetchFieldValues: PropTypes.func.isRequired, +}; + +const FIELD_VALUES_SHOW_LIMIT = 35; + +const mapStateToProps = (state, props) => { + const fieldId = props.field.id; + const fieldValues = + fieldId != null + ? Fields.selectors.getFieldValues(state, { + entityId: fieldId, + }) + : []; + return { + fieldValues: fieldValues || [], + }; +}; + +const mapDispatchToProps = { + fetchFieldValues: Fields.actions.fetchFieldValues, +}; + +export function CategoryFingerprint({ + className, + field, + fieldValues = [], + fetchFieldValues, +}) { + const fieldId = field.id; + const listsFieldValues = field.has_field_values === "list"; + const isMissingFieldValues = fieldValues.length === 0; + const shouldFetchFieldValues = listsFieldValues && isMissingFieldValues; + + const shortenedValuesStr = fieldValues + .slice(0, FIELD_VALUES_SHOW_LIMIT) + .map(value => (Array.isArray(value) ? value[0] : value)) + .join(", "); + + const distinctCount = field.fingerprint?.global?.["distinct-count"]; + const formattedDistinctCount = formatNumber(distinctCount); + + const [isLoading, setIsLoading] = useState(shouldFetchFieldValues); + const safeFetchFieldValues = useAsyncFunction(fetchFieldValues); + useEffect(() => { + if (shouldFetchFieldValues) { + setIsLoading(true); + safeFetchFieldValues({ id: fieldId }).finally(() => { + setIsLoading(false); + }); + } + }, [fieldId, shouldFetchFieldValues, safeFetchFieldValues]); + + const showDistinctCount = isLoading || distinctCount != null; + const showFieldValuesBlock = isLoading || shortenedValuesStr.length > 0; + const showComponent = showDistinctCount || showFieldValuesBlock; + + return showComponent ? ( + <Container className={className}> + {showDistinctCount && ( + <RelativeContainer> + <Fade aria-hidden={!isLoading} visible={!isLoading}> + {ngettext( + msgid`${formattedDistinctCount} distinct value`, + `${formattedDistinctCount} distinct values`, + distinctCount || 0, + )} + </Fade> + <Fade + aria-hidden={!isLoading} + visible={isLoading} + >{t`Getting distinct values...`}</Fade> + </RelativeContainer> + )} + {showFieldValuesBlock && ( + <RelativeContainer height={isLoading && "1.8em"}> + <Fade visible={isLoading}> + <LoadingSpinner /> + </Fade> + <FadeAndSlide visible={!isLoading}> + <NoWrap>{shortenedValuesStr}</NoWrap> + </FadeAndSlide> + </RelativeContainer> + )} + </Container> + ) : null; +} + +CategoryFingerprint.propTypes = propTypes; + +export default connect( + mapStateToProps, + mapDispatchToProps, +)(CategoryFingerprint); diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.styled.jsx b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.styled.jsx new file mode 100644 index 00000000000..26a38b72091 --- /dev/null +++ b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.styled.jsx @@ -0,0 +1,60 @@ +import styled from "styled-components"; + +import { color } from "metabase/lib/colors"; +import _LoadingSpinner from "metabase/components/LoadingSpinner"; +import { isReducedMotionPreferred } from "metabase/lib/dom"; + +const TRANSITION_DURATION = () => (isReducedMotionPreferred() ? "0" : "0.25s"); + +export const Container = styled.div` + font-size: 1em; + position: relative; + display: flex; + flex-direction: column; + gap: 0.6em; + height: auto; + overflow: hidden; +`; + +export const NoWrap = styled.div` + white-space: nowrap; + text-overflow: ellipsis; + overflow: hidden; + font-weight: bold; + padding-top: 0.3em 0; +`; + +export const LoadingSpinner = styled(_LoadingSpinner).attrs({ + size: 18, +})` + display: flex; + flex-grow: 1; + align-self: center; + justify-content: center; + color: ${color("brand")}; +`; + +export const RelativeContainer = styled.div` + position: relative; + height: ${({ height }) => height || "1em"}; + line-height: 1em; +`; + +export const Fade = styled.div` + position: absolute; + top: 0; + left: 0; + width: 100%; + transition: opacity ${TRANSITION_DURATION} linear; + opacity: ${({ visible }) => (visible ? "1" : "0")}; +`; + +export const FadeAndSlide = styled.div` + position: absolute; + width: 100%; + transition: opacity ${TRANSITION_DURATION} linear, + transform ${TRANSITION_DURATION} linear; + opacity: ${({ visible }) => (visible ? "1" : "0")}; + transform: ${({ visible }) => + visible ? "translateY(0)" : "translateY(100%)"}; +`; diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.unit.spec.js b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.unit.spec.js new file mode 100644 index 00000000000..2905d5f3f6e --- /dev/null +++ b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/CategoryFingerprint.unit.spec.js @@ -0,0 +1,124 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; + +import { PRODUCTS, metadata } from "__support__/sample_dataset_fixture"; +import Dimension from "metabase-lib/lib/Dimension"; + +import { CategoryFingerprint } from "./CategoryFingerprint"; + +const categoryField = Dimension.parseMBQL( + ["field", PRODUCTS.CATEGORY.id, null], + metadata, +).field(); + +const mockFetchFieldValues = jest.fn(); +function setup({ + field, + fieldValues, + fingerprint, + fetchFieldValues = mockFetchFieldValues, +}) { + categoryField.fingerprint = fingerprint; + mockFetchFieldValues.mockReset(); + return render( + <CategoryFingerprint + field={field} + fieldValues={fieldValues} + fetchFieldValues={mockFetchFieldValues} + />, + ); +} + +describe("CategoryFingerprint", () => { + describe("when the field does not have a `has_field_values` value of 'list'", () => { + beforeEach(() => { + categoryField.has_field_values = "search"; + }); + + it("should not fetch field values when field values are empty", () => { + setup({ + field: categoryField, + }); + expect(mockFetchFieldValues).not.toHaveBeenCalled(); + }); + + it("should show a distinct count when available", () => { + setup({ + field: categoryField, + fingerprint: { + global: { + "distinct-count": 10, + }, + }, + }); + + expect(screen.getByText("10 distinct values")).toBeVisible(); + }); + + it("should not show a distinct count when the fingerprint value is unavailable", () => { + const { container } = setup({ field: categoryField }); + expect(container.firstChild).toBeNull(); + }); + + it("should show field values if for whatever reason some are present", () => { + setup({ + field: categoryField, + fieldValues: ["foo", "bar"], + }); + expect(screen.getByText("foo, bar")).toBeVisible(); + }); + }); + + describe("when the field has a `has_field_values` value of 'list'", () => { + beforeEach(() => { + categoryField.has_field_values = "list"; + }); + + it("should fetch field values when field values are empty", () => { + setup({ + field: categoryField, + }); + expect(mockFetchFieldValues).toHaveBeenCalledWith({ + id: categoryField.id, + }); + }); + + it("should not fetch field values when field values are presnet", () => { + setup({ + field: categoryField, + fieldValues: ["foo", "bar"], + }); + expect(mockFetchFieldValues).not.toHaveBeenCalled(); + }); + + it("should show a loading state while fetching", () => { + setup({ + field: categoryField, + fetchFieldValues: () => new Promise(), + }); + expect(screen.getByText("Getting distinct values...")).toBeVisible(); + }); + + it("should show a distinct count when available", () => { + setup({ + field: categoryField, + fieldValues: ["foo", "bar"], + fingerprint: { + global: { + "distinct-count": 2, + }, + }, + }); + + expect(screen.getByText("2 distinct values")).toBeVisible(); + }); + + it("should not show a distinct count when the fingerprint value is unavailable", () => { + const { container } = setup({ + field: categoryField, + fieldValues: ["foo"], + }); + expect(container.textContent).toEqual("foo"); + }); + }); +}); diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.jsx b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.jsx index 1098c7db1e6..1dc07fad71c 100644 --- a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.jsx +++ b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.jsx @@ -1,45 +1,51 @@ import React from "react"; import PropTypes from "prop-types"; -import { t, ngettext, msgid } from "ttag"; +import { t } from "ttag"; -import { formatDateTimeWithUnit } from "metabase/lib/formatting"; +import { formatDateTimeWithUnit, formatNumber } from "metabase/lib/formatting"; import Field from "metabase-lib/lib/metadata/Field"; -import FieldValuesList from "./FieldValuesList"; +import { Table } from "../MetadataInfo.styled"; +import CategoryFingerprint from "./CategoryFingerprint"; const propTypes = { + className: PropTypes.string, field: PropTypes.instanceOf(Field), }; -function FieldFingerprintInfo({ field }) { +function FieldFingerprintInfo({ className, field }) { if (!field?.fingerprint) { return null; } if (field.isDate()) { - return <DateTimeFingerprint field={field} />; + return <DateTimeFingerprint className={className} field={field} />; } else if (field.isNumber()) { - return <NumberFingerprint field={field} />; + return <NumberFingerprint className={className} field={field} />; } else if (field.isCategory()) { - return <CategoryFingerprint field={field} />; + return <CategoryFingerprint className={className} field={field} />; } else { return null; } } -function DateTimeFingerprint({ field }) { +function getTimezone(field) { + return field.query?.database?.()?.timezone || field.table?.database?.timezone; +} + +function DateTimeFingerprint({ className, field }) { const dateTimeFingerprint = field.fingerprint.type["type/DateTime"]; if (!dateTimeFingerprint) { return null; } - const timezone = field?.table?.database?.timezone; + const timezone = getTimezone(field); const { earliest, latest } = dateTimeFingerprint; const formattedEarliest = formatDateTimeWithUnit(earliest, "minute"); const formattedLatest = formatDateTimeWithUnit(latest, "minute"); return ( - <table> + <Table className={className}> <tbody> <tr> <th>{t`Timezone`}</th> @@ -54,23 +60,23 @@ function DateTimeFingerprint({ field }) { <td>{formattedLatest}</td> </tr> </tbody> - </table> + </Table> ); } -function NumberFingerprint({ field }) { +function NumberFingerprint({ className, field }) { const numberFingerprint = field.fingerprint.type["type/Number"]; if (!numberFingerprint) { return null; } const { avg, min, max } = numberFingerprint; - const fixedAvg = Number.isInteger(avg) ? avg : avg.toFixed(2); - const fixedMin = Number.isInteger(min) ? min : min.toFixed(2); - const fixedMax = Number.isInteger(max) ? max : max.toFixed(2); + const fixedAvg = formatNumber(Number.isInteger(avg) ? avg : avg.toFixed(2)); + const fixedMin = formatNumber(Number.isInteger(min) ? min : min.toFixed(2)); + const fixedMax = formatNumber(Number.isInteger(max) ? max : max.toFixed(2)); return ( - <table> + <Table className={className}> <thead> <tr> <th>{t`Average`}</th> @@ -85,33 +91,12 @@ function NumberFingerprint({ field }) { <td>{fixedMax}</td> </tr> </tbody> - </table> - ); -} - -function CategoryFingerprint({ field }) { - const distinctCount = field.fingerprint.global?.["distinct-count"]; - if (distinctCount == null) { - return null; - } - - return ( - <div> - <div> - {ngettext( - msgid`${distinctCount} distinct value`, - `${distinctCount} distinct values`, - distinctCount, - )} - </div> - <FieldValuesList field={field} /> - </div> + </Table> ); } FieldFingerprintInfo.propTypes = propTypes; DateTimeFingerprint.propTypes = propTypes; NumberFingerprint.propTypes = propTypes; -CategoryFingerprint.propTypes = propTypes; export default FieldFingerprintInfo; diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.unit.spec.js b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.unit.spec.js index 814fad1ea99..7eabd162187 100644 --- a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.unit.spec.js +++ b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldFingerprintInfo.unit.spec.js @@ -124,42 +124,6 @@ describe("FieldFingerprintInfo", () => { }); }); - describe("Category field", () => { - const categoryField = Dimension.parseMBQL( - ["field", PRODUCTS.CATEGORY.id, null], - metadata, - ).field(); - let container; - - describe("without type/Category fingerprint", () => { - beforeEach(() => { - categoryField.fingerprint = { type: {} }; - const wrapper = setup(categoryField); - container = wrapper.container; - }); - - it("should render nothing", () => { - expect(container.firstChild).toBeNull(); - }); - }); - - describe("with type/Category fingerprint", () => { - beforeEach(() => { - categoryField.fingerprint = { - global: { - "distinct-count": 123, - }, - }; - - setup(categoryField); - }); - - it("should render the distinct count value", () => { - expect(screen.getByText("123 distinct values")).toBeVisible(); - }); - }); - }); - describe("Other field types", () => { it("should render nothing", () => { const idField = Dimension.parseMBQL( diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.jsx b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.jsx deleted file mode 100644 index c0b331e2275..00000000000 --- a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.jsx +++ /dev/null @@ -1,44 +0,0 @@ -import React, { useEffect } from "react"; -import PropTypes from "prop-types"; -import { connect } from "react-redux"; - -import Fields from "metabase/entities/fields"; -import Field from "metabase-lib/lib/metadata/Field"; - -import { NoWrap } from "./FieldValuesList.styled"; - -const mapStateToProps = (state, props) => ({ - fieldValues: Fields.selectors.getFieldValues(state, { - entityId: props.field.id, - }), -}); - -const mapDispatchToProps = { - fetchFieldValues: Fields.actions.fetchFieldValues, -}; - -const propTypes = { - field: PropTypes.instanceOf(Field).isRequired, - fieldValues: PropTypes.array, - fetchFieldValues: PropTypes.func.isRequired, -}; - -const FIELD_VALUES_SHOW_LIMIT = 35; - -export function FieldValuesList({ field, fieldValues = [], fetchFieldValues }) { - useEffect(() => { - if (fieldValues.length === 0 && field.has_field_values === "list") { - fetchFieldValues({ id: field.id }); - } - }, [fetchFieldValues, field, fieldValues]); - - const shortenedValuesStr = fieldValues - .slice(0, FIELD_VALUES_SHOW_LIMIT) - .join(", "); - - return fieldValues.length ? <NoWrap>{shortenedValuesStr}</NoWrap> : null; -} - -FieldValuesList.propTypes = propTypes; - -export default connect(mapStateToProps, mapDispatchToProps)(FieldValuesList); diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.styled.jsx b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.styled.jsx deleted file mode 100644 index 33d6702a93d..00000000000 --- a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.styled.jsx +++ /dev/null @@ -1,11 +0,0 @@ -import styled from "styled-components"; - -import { space } from "metabase/styled-components/theme"; - -export const NoWrap = styled.div` - white-space: nowrap; - text-overflow: ellipsis; - overflow: hidden; - font-weight: bold; - padding-top: ${space(0)} 0; -`; diff --git a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.unit.spec.js b/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.unit.spec.js deleted file mode 100644 index b5523a3a6f3..00000000000 --- a/frontend/src/metabase/components/MetadataInfo/FieldFingerprintInfo/FieldValuesList.unit.spec.js +++ /dev/null @@ -1,68 +0,0 @@ -import React from "react"; -import { render, screen } from "@testing-library/react"; - -import { PRODUCTS, metadata } from "__support__/sample_dataset_fixture"; -import Dimension from "metabase-lib/lib/Dimension"; - -import { FieldValuesList } from "./FieldValuesList"; - -const categoryField = Dimension.parseMBQL( - ["field", PRODUCTS.CATEGORY.id, null], - metadata, -).field(); - -const mockFetchFieldValues = jest.fn(); -function setup(field, fieldValues) { - mockFetchFieldValues.mockReset(); - return render( - <FieldValuesList - field={field} - fieldValues={fieldValues} - fetchFieldValues={mockFetchFieldValues} - />, - ); -} - -describe("FieldValuesList", () => { - it("should return nothing when there are no values", () => { - const { container } = setup(categoryField, []); - expect(container.firstChild).toBeNull(); - }); - - it("should return a formatted values list", () => { - setup(categoryField, ["foo", "bar"]); - expect(screen.getByText("foo, bar")).toBeInTheDocument(); - }); - - it("should return a shortened, formatted values list", () => { - setup(categoryField, Array(50).fill("a")); - expect( - screen.getByText( - Array(35) - .fill("a") - .join(", "), - ), - ).toBeInTheDocument(); - }); - - it("should fetch field values when `fieldValues` is empty and the field is set to list values", () => { - categoryField.has_field_values = "list"; - setup(categoryField, []); - - expect(mockFetchFieldValues).toHaveBeenCalledWith({ id: categoryField.id }); - }); - - it("should not fetch field values when `fieldValues` is not empty", () => { - categoryField.has_field_values = "list"; - setup(categoryField, ["hey"]); - - expect(mockFetchFieldValues).not.toHaveBeenCalled(); - }); - - it("should not fetch field values when the field is not set to list values", () => { - categoryField.has_field_values = "search"; - setup(categoryField, [""]); - - expect(mockFetchFieldValues).not.toHaveBeenCalled(); - }); -}); diff --git a/frontend/src/metabase/components/MetadataInfo/MetadataInfo.styled.tsx b/frontend/src/metabase/components/MetadataInfo/MetadataInfo.styled.tsx index 41988704b03..68303536af5 100644 --- a/frontend/src/metabase/components/MetadataInfo/MetadataInfo.styled.tsx +++ b/frontend/src/metabase/components/MetadataInfo/MetadataInfo.styled.tsx @@ -1,6 +1,5 @@ import styled from "styled-components"; -import { space } from "metabase/styled-components/theme"; import { color } from "metabase/lib/colors"; import Icon from "metabase/components/Icon"; import { isReducedMotionPreferred } from "metabase/lib/dom"; @@ -12,7 +11,7 @@ export const Container = styled.div` position: relative; display: flex; flex-direction: column; - gap: ${space(1)}; + gap: 0.8em; overflow: auto; `; @@ -28,11 +27,10 @@ export const AbsoluteContainer = styled.div` `; export const InfoContainer = styled(Container)` - padding: ${space(2)}; + padding: 1.1em; `; export const Description = styled.div` - font-size: 14px; white-space: pre-line; max-height: 200px; overflow: auto; @@ -46,14 +44,15 @@ export const EmptyDescription = styled(Description)` export const LabelContainer = styled.div` display: inline-flex; align-items: center; - column-gap: ${space(0)}; - font-size: 12px; + column-gap: 0.3em; + font-size: 1em; color: ${({ color: _color = "brand" }) => color(_color)}; `; export const Label = styled.span` font-weight: bold; font-size: 1em; + line-height: 1em; `; export const RelativeSizeIcon = styled(Icon)` @@ -62,11 +61,10 @@ export const RelativeSizeIcon = styled(Icon)` `; export const InvertedColorRelativeSizeIcon = styled(RelativeSizeIcon)` - padding: ${space(0)}; background-color: ${color("brand")}; color: ${color("white")}; - border-radius: ${space(0)}; - padding: ${space(0)}; + border-radius: 0.3em; + padding: 0.3em; `; type FadeProps = { @@ -77,13 +75,10 @@ export const Fade = styled.div<FadeProps>` width: 100%; transition: opacity ${TRANSITION_DURATION} linear; opacity: ${({ visible }) => (visible ? "1" : "0")}; -`; -export const FadeAndSlide = styled.div<FadeProps>` - width: 100%; - transition: opacity ${TRANSITION_DURATION} linear, - transform ${TRANSITION_DURATION} linear; - opacity: ${({ visible }) => (visible ? "1" : "0")}; + &:empty { + display: none; + } `; export const LoadingSpinner = styled(_LoadingSpinner)` @@ -93,3 +88,15 @@ export const LoadingSpinner = styled(_LoadingSpinner)` justify-content: center; color: ${color("brand")}; `; + +export const Table = styled.table` + font-size: 1em; + + th { + font-weight: normal; + } + + td { + font-weight: bold; + } +`; diff --git a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.styled.tsx b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.styled.tsx new file mode 100644 index 00000000000..09d141d6e5b --- /dev/null +++ b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.styled.tsx @@ -0,0 +1,14 @@ +import styled from "styled-components"; + +import { + InfoContainer as _InfoContainer, + Container, +} from "metabase/components/MetadataInfo/MetadataInfo.styled"; + +export const MetadataContainer = styled(Container)` + font-size: 0.9em; +`; + +export const InfoContainer = styled(_InfoContainer)` + gap: 0.2em; +`; diff --git a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx index b7d534819da..778b5dc9060 100644 --- a/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx +++ b/frontend/src/metabase/components/MetadataInfo/TableInfo/TableInfo.tsx @@ -8,14 +8,13 @@ import Tables from "metabase/entities/tables"; import Table from "metabase-lib/lib/metadata/Table"; import { - InfoContainer, Description, EmptyDescription, LoadingSpinner, AbsoluteContainer, Fade, - Container, } from "../MetadataInfo.styled"; +import { InfoContainer, MetadataContainer } from "./TableInfo.styled"; import ColumnCount from "./ColumnCount"; import ConnectedTables from "./ConnectedTables"; @@ -102,7 +101,7 @@ export function TableInfo({ ) : ( <EmptyDescription>{t`No description`}</EmptyDescription> )} - <Container> + <MetadataContainer> <Fade visible={!hasFetchedMetadata}> <AbsoluteContainer> <LoadingSpinner size={24} /> @@ -114,7 +113,7 @@ export function TableInfo({ <Fade visible={hasFetchedMetadata}> <ConnectedTables table={table} /> </Fade> - </Container> + </MetadataContainer> </InfoContainer> ); } diff --git a/frontend/src/metabase/visualizations/components/TableInteractive.jsx b/frontend/src/metabase/visualizations/components/TableInteractive.jsx index bfe699c580f..9585a7b942e 100644 --- a/frontend/src/metabase/visualizations/components/TableInteractive.jsx +++ b/frontend/src/metabase/visualizations/components/TableInteractive.jsx @@ -30,6 +30,7 @@ import MiniBar from "./MiniBar"; import { Grid, ScrollSync } from "react-virtualized"; import Draggable from "react-draggable"; import Ellipsified from "metabase/components/Ellipsified"; +import DimensionInfoPopover from "metabase/components/MetadataInfo/DimensionInfoPopover"; const HEADER_HEIGHT = 36; const ROW_HEIGHT = 36; @@ -522,6 +523,21 @@ export default class TableInteractive extends Component { return style.left; } + @memoize + getDimension(column, query) { + if (!query) { + return undefined; + } + + const dimension = Dimension.parseMBQL( + column.field_ref, + query.metadata(), + query, + ); + + return dimension; + } + tableHeaderRenderer = ({ key, style, columnIndex }) => { const { data, @@ -631,31 +647,39 @@ export default class TableInteractive extends Component { : undefined } > - {renderTableHeaderWrapper( - <Ellipsified tooltip={columnTitle}> - {isSortable && isRightAligned && ( - <Icon - className="Icon mr1" - name={isAscending ? "chevronup" : "chevrondown"} - size={8} - /> - )} - {columnTitle} - {isSortable && !isRightAligned && ( - <Icon - className="Icon ml1" - name={isAscending ? "chevronup" : "chevrondown"} - size={8} - /> - )} - </Ellipsified>, - column, - columnIndex, - )} + <DimensionInfoPopover + placement="bottom-start" + dimension={this.getDimension(column, this.props.query)} + > + {renderTableHeaderWrapper( + <Ellipsified tooltip={columnTitle}> + {isSortable && isRightAligned && ( + <Icon + className="Icon mr1" + name={isAscending ? "chevronup" : "chevrondown"} + size={8} + /> + )} + {columnTitle} + {isSortable && !isRightAligned && ( + <Icon + className="Icon ml1" + name={isAscending ? "chevronup" : "chevrondown"} + size={8} + /> + )} + </Ellipsified>, + column, + columnIndex, + )} + </DimensionInfoPopover> <Draggable axis="x" bounds={{ left: RESIZE_HANDLE_WIDTH }} - position={{ x: this.getColumnWidth({ index: columnIndex }), y: 0 }} + position={{ + x: this.getColumnWidth({ index: columnIndex }), + y: 0, + }} onStart={e => { e.stopPropagation(); this.setState({ dragColIndex: columnIndex }); diff --git a/frontend/test/metabase/scenarios/visualizations/table.cy.spec.js b/frontend/test/metabase/scenarios/visualizations/table.cy.spec.js index bba2b006f23..f67fa9fabe4 100644 --- a/frontend/test/metabase/scenarios/visualizations/table.cy.spec.js +++ b/frontend/test/metabase/scenarios/visualizations/table.cy.spec.js @@ -1,4 +1,11 @@ -import { restore, openPeopleTable, popover } from "__support__/e2e/cypress"; +import { + restore, + openPeopleTable, + openNativeEditor, + popover, + enterCustomColumnDetails, + visualize, +} from "__support__/e2e/cypress"; describe("scenarios > visualizations > table", () => { beforeEach(() => { @@ -44,6 +51,123 @@ describe("scenarios > visualizations > table", () => { ); }); + it("should show field metadata in a popover when hovering over a table column header", () => { + openPeopleTable(); + cy.wait("@dataset"); + + cy.icon("notebook").click(); + cy.findByTestId("fields-picker").click(); + popover().within(() => { + cy.findByText("Select none").click(); + cy.findByText("City").click(); + cy.findByText("State").click(); + cy.findByText("Birth Date").click(); + cy.findByText("Latitude").click(); + }); + + cy.findByText("Custom column").click(); + + popover().within(() => { + enterCustomColumnDetails({ + formula: "concat([Name], [Name])", + name: "CustomColumn", + }); + + cy.findByText("Done").click(); + }); + + visualize(); + + [ + [ + "ID", + () => { + cy.contains("A unique identifier given to each user."); + }, + ], + [ + "City", + () => { + cy.contains("The city of the account’s billing address"); + cy.findByText("1,966 distinct values"); + }, + ], + [ + "State", + () => { + cy.findByText("49 distinct values"); + cy.contains("AK, AL, AR"); + }, + ], + [ + "Birth Date", + () => { + cy.findByText("America/Los_Angeles"); + cy.findByText("April 26, 1958, 12:00 AM"); + cy.findByText("April 3, 2000, 12:00 AM"); + }, + ], + [ + "Latitude", + () => { + cy.contains("39.88"); + cy.findByText("25.78"); + cy.findByText("70.64"); + }, + ], + [ + "CustomColumn", + () => { + cy.findByText("No description"); + }, + ], + ].forEach(([column, test]) => { + cy.get(".cellData") + .contains(column) + .trigger("mouseenter"); + + popover().within(() => { + cy.contains(column); + test(); + }); + + cy.get(".cellData") + .contains(column) + .trigger("mouseleave"); + }); + + cy.findAllByText("Summarize") + .first() + .click(); + cy.findAllByTestId("dimension-list-item-name") + .first() + .click(); + + cy.icon("table2").click(); + + cy.get(".cellData") + .contains("Count") + .trigger("mouseenter"); + + popover().within(() => { + cy.contains("Count"); + cy.findByText("No description"); + }); + }); + + it("should show field metadata popovers for native query tables", () => { + openNativeEditor().type("select * from products"); + cy.get(".NativeQueryEditor .Icon-play").click(); + + cy.get(".cellData") + .contains("CATEGORY") + .trigger("mouseenter"); + popover().within(() => { + cy.contains("CATEGORY"); + cy.findByText("No description"); + }); + }); + it.skip("should close the colum popover on subsequent click (metabase#16789)", () => { openPeopleTable(); cy.wait("@dataset"); -- GitLab