From 9ce72a44cd1214019088379987b5bb903a5007f1 Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Wed, 26 Jun 2024 11:31:16 +0100
Subject: [PATCH] Bump dashboard description max length limit to 1.5K (#44470)
* Bump dashboard description max len limit to 1.5K
* Extract `DASHBOARD_DESCRIPTION_MAX_LENGTH` const
* Handle description max length in dashboard sidebar
* Add red border to dashboard description box
* Add test
---
.../components/EditableText/EditableText.tsx | 31 +++++++++-------
.../DashboardInfoSidebar.styled.tsx | 13 +++++++
.../DashboardInfoSidebar.tsx | 36 +++++++++++++++----
.../DashboardInfoSidebar.unit.spec.tsx | 27 ++++++++++++++
frontend/src/metabase/dashboard/constants.ts | 2 ++
.../containers/CopyDashboardForm.tsx | 6 +++-
.../containers/CreateDashboardForm.tsx | 7 +++-
7 files changed, 101 insertions(+), 21 deletions(-)
diff --git a/frontend/src/metabase/core/components/EditableText/EditableText.tsx b/frontend/src/metabase/core/components/EditableText/EditableText.tsx
index 80656987df0..f206a4ed17e 100644
--- a/frontend/src/metabase/core/components/EditableText/EditableText.tsx
+++ b/frontend/src/metabase/core/components/EditableText/EditableText.tsx
@@ -4,6 +4,8 @@ import type {
HTMLAttributes,
Ref,
MouseEvent,
+ FocusEventHandler,
+ FocusEvent,
} from "react";
import { forwardRef, useCallback, useEffect, useState, useRef } from "react";
import { usePrevious } from "react-use";
@@ -14,7 +16,7 @@ import { EditableTextArea, EditableTextRoot } from "./EditableText.styled";
export type EditableTextAttributes = Omit<
HTMLAttributes<HTMLDivElement>,
- "onChange"
+ "onChange" | "onFocus" | "onBlur"
>;
export interface EditableTextProps extends EditableTextAttributes {
@@ -26,8 +28,8 @@ export interface EditableTextProps extends EditableTextAttributes {
isDisabled?: boolean;
isMarkdown?: boolean;
onChange?: (value: string) => void;
- onFocus?: () => void;
- onBlur?: () => void;
+ onFocus?: FocusEventHandler<HTMLTextAreaElement>;
+ onBlur?: FocusEventHandler<HTMLTextAreaElement>;
"data-testid"?: string;
}
@@ -72,18 +74,21 @@ const EditableText = forwardRef(function EditableText(
}
}, [isInFocus, isMarkdown]);
- const handleBlur = useCallback(() => {
- setIsInFocus(false);
+ const handleBlur = useCallback(
+ (event: FocusEvent<HTMLTextAreaElement>) => {
+ setIsInFocus(false);
- if (!isOptional && !inputValue) {
- setInputValue(submitValue);
- } else if (inputValue !== submitValue && submitOnBlur.current) {
- setSubmitValue(inputValue);
- onChange?.(inputValue);
- }
+ if (!isOptional && !inputValue) {
+ setInputValue(submitValue);
+ } else if (inputValue !== submitValue && submitOnBlur.current) {
+ setSubmitValue(inputValue);
+ onChange?.(inputValue);
+ }
- onBlur?.();
- }, [inputValue, submitValue, isOptional, onChange, onBlur, setIsInFocus]);
+ onBlur?.(event);
+ },
+ [inputValue, submitValue, isOptional, onChange, onBlur, setIsInFocus],
+ );
const handleChange = useCallback(
(event: ChangeEvent<HTMLTextAreaElement>) => {
diff --git a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.styled.tsx b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.styled.tsx
index 7082d0e53df..fb606664b92 100644
--- a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.styled.tsx
+++ b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.styled.tsx
@@ -1,3 +1,4 @@
+import { css } from "@emotion/react";
import styled from "@emotion/styled";
import EditableText from "metabase/core/components/EditableText";
@@ -61,3 +62,15 @@ export const ContentSection = styled.div`
export const DescriptionHeader = styled.h3`
margin-bottom: 0.5rem;
`;
+
+export const EditableDescription = styled(EditableText)<{ hasError?: boolean }>`
+ ${props =>
+ props.hasError &&
+ css`
+ border-color: var(--mb-color-error);
+
+ &:hover {
+ border-color: var(--mb-color-error);
+ }
+ `}
+`;
diff --git a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx
index 1f3768f1421..177e507a17c 100644
--- a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx
+++ b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx
@@ -1,4 +1,4 @@
-import type { Dispatch, SetStateAction } from "react";
+import type { Dispatch, FocusEvent, SetStateAction } from "react";
import { useCallback, useState } from "react";
import { t } from "ttag";
@@ -6,24 +6,25 @@ import ErrorBoundary from "metabase/ErrorBoundary";
import { Timeline } from "metabase/common/components/Timeline";
import { getTimelineEvents } from "metabase/common/components/Timeline/utils";
import { useRevisionListQuery } from "metabase/common/hooks";
-import EditableText from "metabase/core/components/EditableText";
import {
revertToRevision,
toggleAutoApplyFilters,
updateDashboard,
} from "metabase/dashboard/actions";
+import { DASHBOARD_DESCRIPTION_MAX_LENGTH } from "metabase/dashboard/constants";
import { isDashboardCacheable } from "metabase/dashboard/utils";
import { useUniqueId } from "metabase/hooks/use-unique-id";
import { useDispatch, useSelector } from "metabase/lib/redux";
import { PLUGIN_CACHING } from "metabase/plugins";
import { getUser } from "metabase/selectors/user";
-import { Stack, Switch } from "metabase/ui";
+import { Text, Stack, Switch } from "metabase/ui";
import type { Dashboard } from "metabase-types/api";
import {
ContentSection,
DashboardInfoSidebarRoot,
DescriptionHeader,
+ EditableDescription,
HistoryHeader,
} from "./DashboardInfoSidebar.styled";
@@ -75,6 +76,8 @@ const DashboardInfoSidebarBody = ({
setDashboardAttribute,
setPage,
}: DashboardSidebarPageProps) => {
+ const [descriptionError, setDescriptionError] = useState<string | null>(null);
+
const { data: revisions } = useRevisionListQuery({
query: { model_type: "dashboard", model_id: dashboard.id },
});
@@ -84,12 +87,25 @@ const DashboardInfoSidebarBody = ({
const handleDescriptionChange = useCallback(
(description: string) => {
- setDashboardAttribute?.("description", description);
- dispatch(updateDashboard({ attributeNames: ["description"] }));
+ if (description.length <= DASHBOARD_DESCRIPTION_MAX_LENGTH) {
+ setDashboardAttribute?.("description", description);
+ dispatch(updateDashboard({ attributeNames: ["description"] }));
+ }
},
[dispatch, setDashboardAttribute],
);
+ const handleDescriptionBlur = useCallback(
+ (event: FocusEvent<HTMLTextAreaElement>) => {
+ if (event.target.value.length > DASHBOARD_DESCRIPTION_MAX_LENGTH) {
+ setDescriptionError(
+ t`Must be ${DASHBOARD_DESCRIPTION_MAX_LENGTH} characters or less`,
+ );
+ }
+ },
+ [],
+ );
+
const handleToggleAutoApplyFilters = useCallback(
(isAutoApplyingFilters: boolean) => {
dispatch(toggleAutoApplyFilters(isAutoApplyingFilters));
@@ -107,17 +123,25 @@ const DashboardInfoSidebarBody = ({
<>
<ContentSection>
<DescriptionHeader>{t`About`}</DescriptionHeader>
- <EditableText
+ <EditableDescription
initialValue={dashboard.description}
isDisabled={!canWrite}
onChange={handleDescriptionChange}
+ onFocus={() => setDescriptionError("")}
+ onBlur={handleDescriptionBlur}
isOptional
isMultiline
isMarkdown
+ hasError={!!descriptionError}
placeholder={t`Add description`}
key={`dashboard-description-${dashboard.description}`}
style={{ fontSize: ".875rem" }}
/>
+ {!!descriptionError && (
+ <Text color="error" size="xs" mt="xs">
+ {descriptionError}
+ </Text>
+ )}
</ContentSection>
{!dashboard.archived && (
diff --git a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.unit.spec.tsx b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.unit.spec.tsx
index 1022d5eba89..816a2574b22 100644
--- a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.unit.spec.tsx
+++ b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.unit.spec.tsx
@@ -28,6 +28,11 @@ function setup({ dashboard = createMockDashboard() }: SetupOpts = {}) {
};
}
+jest.mock("metabase/dashboard/constants", () => ({
+ ...jest.requireActual("metabase/dashboard/constants"),
+ DASHBOARD_DESCRIPTION_MAX_LENGTH: 20,
+}));
+
describe("DashboardInfoSidebar", () => {
it("should render the component", () => {
setup();
@@ -51,6 +56,28 @@ describe("DashboardInfoSidebar", () => {
);
});
+ it("should validate description length", async () => {
+ const expectedErrorMessage = "Must be 20 characters or less";
+ const { setDashboardAttribute } = setup();
+
+ await userEvent.click(screen.getByTestId("editable-text"));
+ await userEvent.type(
+ screen.getByPlaceholderText("Add description"),
+ "in incididunt incididunt laboris ut elit culpa sit dolor amet",
+ );
+ await userEvent.tab();
+
+ expect(screen.getByText(expectedErrorMessage)).toBeInTheDocument();
+
+ await userEvent.click(screen.getByTestId("editable-text"));
+ expect(screen.queryByText(expectedErrorMessage)).not.toBeInTheDocument();
+
+ await userEvent.tab();
+ expect(screen.getByText(expectedErrorMessage)).toBeInTheDocument();
+
+ expect(setDashboardAttribute).not.toHaveBeenCalled();
+ });
+
it("should allow to clear description", async () => {
const { setDashboardAttribute } = setup({
dashboard: createMockDashboard({ description: "some description" }),
diff --git a/frontend/src/metabase/dashboard/constants.ts b/frontend/src/metabase/dashboard/constants.ts
index b2a80a4fb4c..a974f3aea89 100644
--- a/frontend/src/metabase/dashboard/constants.ts
+++ b/frontend/src/metabase/dashboard/constants.ts
@@ -5,6 +5,8 @@ import type {
import type { EmbedDisplayParams } from "./types";
+export const DASHBOARD_DESCRIPTION_MAX_LENGTH = 1500;
+
export const SIDEBAR_NAME: Record<DashboardSidebarName, DashboardSidebarName> =
{
addQuestion: "addQuestion",
diff --git a/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx b/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx
index ef2c256f627..6df4e2ce5e4 100644
--- a/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx
+++ b/frontend/src/metabase/dashboard/containers/CopyDashboardForm.tsx
@@ -23,13 +23,17 @@ import * as Errors from "metabase/lib/errors";
import type { CollectionId, Dashboard } from "metabase-types/api";
import { DashboardCopyModalShallowCheckboxLabel } from "../components/DashboardCopyModal/DashboardCopyModalShallowCheckboxLabel/DashboardCopyModalShallowCheckboxLabel";
+import { DASHBOARD_DESCRIPTION_MAX_LENGTH } from "../constants";
const DASHBOARD_SCHEMA = Yup.object({
name: Yup.string()
.required(Errors.required)
.max(100, Errors.maxLength)
.default(""),
- description: Yup.string().nullable().max(255, Errors.maxLength).default(null),
+ description: Yup.string()
+ .nullable()
+ .max(DASHBOARD_DESCRIPTION_MAX_LENGTH, Errors.maxLength)
+ .default(null),
collection_id: Yup.number().nullable().default(null),
is_shallow_copy: Yup.boolean().default(false),
});
diff --git a/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx b/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx
index 44d432fe5e3..2cfaa326ce1 100644
--- a/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx
+++ b/frontend/src/metabase/dashboard/containers/CreateDashboardForm.tsx
@@ -20,12 +20,17 @@ import * as Errors from "metabase/lib/errors";
import type { CollectionId, Dashboard } from "metabase-types/api";
import type { State } from "metabase-types/store";
+import { DASHBOARD_DESCRIPTION_MAX_LENGTH } from "../constants";
+
const DASHBOARD_SCHEMA = Yup.object({
name: Yup.string()
.required(Errors.required)
.max(100, Errors.maxLength)
.default(""),
- description: Yup.string().nullable().max(255, Errors.maxLength).default(null),
+ description: Yup.string()
+ .nullable()
+ .max(DASHBOARD_DESCRIPTION_MAX_LENGTH, Errors.maxLength)
+ .default(null),
collection_id: Yup.number().nullable(),
});
--
GitLab