From a8b14f29d185a16dfa4dfd675b9f1dc9e2212d8a Mon Sep 17 00:00:00 2001 From: Tim Conkling Date: Tue, 10 Nov 2020 19:19:18 -0800 Subject: [PATCH] takes a body and format, rather than an AlertProto (#2335) This means that when we want to show an Alert that did not originate from a protobuf (for example, various "Loading..." alerts), we can do so without wrapping body/format inside a superfluous object. --- frontend/src/components/core/Block/Block.tsx | 21 +++++----- .../components/elements/Alert/Alert.test.tsx | 38 +++++++------------ .../src/components/elements/Alert/Alert.tsx | 16 ++++---- .../hocs/withMapboxToken/withMapboxToken.tsx | 17 +++------ 4 files changed, 40 insertions(+), 52 deletions(-) diff --git a/frontend/src/components/core/Block/Block.tsx b/frontend/src/components/core/Block/Block.tsx index 3f4b566a8b2c..5042d997ba5c 100644 --- a/frontend/src/components/core/Block/Block.tsx +++ b/frontend/src/components/core/Block/Block.tsx @@ -54,13 +54,15 @@ import { styled, StyletronComponent } from "styletron-react" import debounceRender from "react-debounce-render" import { ReportRunState } from "lib/ReportRunState" import { WidgetStateManager } from "lib/WidgetStateManager" -import { getElementWidgetID, makeElementWithInfoText } from "lib/utils" +import { getElementWidgetID } from "lib/utils" import { FileUploadClient } from "lib/FileUploadClient" import { variables as stylingVariables } from "lib/widgetTheme" import { BlockNode, ReportNode, ElementNode } from "lib/ReportNode" // Load (non-lazy) elements. import Alert from "components/elements/Alert/" +import { getAlertKind } from "components/elements/Alert/Alert" +import { Kind } from "components/shared/AlertContainer" import DocString from "components/elements/DocString/" import ErrorBoundary from "components/shared/ErrorBoundary/" import FullScreenWrapper from "components/shared/FullScreenWrapper/" @@ -298,12 +300,7 @@ class Block extends PureComponent { + } > {element} @@ -347,10 +344,16 @@ class Block extends PureComponent { } switch (node.element.type) { - case "alert": + case "alert": { + const alertProto = node.element.alert as AlertProto return ( - + ) + } case "audio": return ( diff --git a/frontend/src/components/elements/Alert/Alert.test.tsx b/frontend/src/components/elements/Alert/Alert.test.tsx index 5835be6ffcb6..81ab6128cd1f 100644 --- a/frontend/src/components/elements/Alert/Alert.test.tsx +++ b/frontend/src/components/elements/Alert/Alert.test.tsx @@ -20,21 +20,19 @@ import React from "react" import { shallow } from "lib/test_util" import { Kind } from "components/shared/AlertContainer" import { Alert as AlertProto } from "autogen/proto" -import Alert, { AlertProps } from "./Alert" +import Alert, { AlertProps, getAlertKind } from "./Alert" -const getProps = (elementProps: Partial = {}): AlertProps => ({ - element: AlertProto.create({ - body: "Something happened!", - ...elementProps, - }), +const getProps = (elementProps: Partial = {}): AlertProps => ({ + body: "Something happened!", + kind: Kind.INFO, width: 100, + ...elementProps, }) describe("Alert element", () => { it("renders an ERROR box as expected", () => { - const format = AlertProto.Format.ERROR const props = getProps({ - format, + kind: getAlertKind(AlertProto.Format.ERROR), body: "#what in the world?", }) const wrap = shallow() @@ -47,9 +45,8 @@ describe("Alert element", () => { }) it("renders a WARNING box as expected", () => { - const format = AlertProto.Format.WARNING const props = getProps({ - format, + kind: getAlertKind(AlertProto.Format.WARNING), body: "Are you *sure*?", }) const wrap = shallow() @@ -62,9 +59,8 @@ describe("Alert element", () => { }) it("renders a SUCCESS box as expected", () => { - const format = AlertProto.Format.SUCCESS const props = getProps({ - format, + kind: getAlertKind(AlertProto.Format.SUCCESS), body: "But our princess was in another castle!", }) const wrap = shallow() @@ -77,9 +73,8 @@ describe("Alert element", () => { }) it("renders an INFO box as expected", () => { - const format = AlertProto.Format.INFO const props = getProps({ - format, + kind: getAlertKind(AlertProto.Format.INFO), body: "It's dangerous to go alone.", }) const wrap = shallow() @@ -90,15 +85,10 @@ describe("Alert element", () => { "It's dangerous to go alone." ) }) +}) - it("should throw an error when the format is invalid", () => { - const props = getProps({ - format: ("test" as unknown) as AlertProto.Format, - body: "It's dangerous to go alone.", - }) - - expect(() => { - shallow() - }).toThrow("Unexpected alert type: test") - }) +test("getAlertKind throws an error on invalid format", () => { + expect(() => getAlertKind(AlertProto.Format.UNUSED)).toThrow( + `Unexpected alert type: ${AlertProto.Format.UNUSED}` + ) }) diff --git a/frontend/src/components/elements/Alert/Alert.tsx b/frontend/src/components/elements/Alert/Alert.tsx index 0db4085d5da2..50aedd7aed93 100644 --- a/frontend/src/components/elements/Alert/Alert.tsx +++ b/frontend/src/components/elements/Alert/Alert.tsx @@ -37,20 +37,22 @@ export function getAlertKind(format: AlertProto.Format): Kind { } export interface AlertProps { + body: string + kind: Kind width: number - element: AlertProto } /** - * Functional element representing error/warning/info/success boxes - * which may be formatted in Markdown. + * Display an (error|warning|info|success) box with a Markdown-formatted body. */ -export default function Alert({ element, width }: AlertProps): ReactElement { - const { body, format } = element - +export default function Alert({ + body, + kind, + width, +}: AlertProps): ReactElement { return (
- +
diff --git a/frontend/src/hocs/withMapboxToken/withMapboxToken.tsx b/frontend/src/hocs/withMapboxToken/withMapboxToken.tsx index 165da53a806d..248cf7b117f2 100644 --- a/frontend/src/hocs/withMapboxToken/withMapboxToken.tsx +++ b/frontend/src/hocs/withMapboxToken/withMapboxToken.tsx @@ -15,13 +15,11 @@ * limitations under the License. */ -import React, { ComponentType, PureComponent } from "react" -import { makeElementWithInfoText } from "lib/utils" -import hoistNonReactStatics from "hoist-non-react-statics" -import { MapboxToken } from "hocs/withMapboxToken/MapboxToken" - -import { Alert as AlertProto } from "autogen/proto" import Alert from "components/elements/Alert" +import { Kind } from "components/shared/AlertContainer" +import { MapboxToken } from "hocs/withMapboxToken/MapboxToken" +import hoistNonReactStatics from "hoist-non-react-statics" +import React, { ComponentType, PureComponent } from "react" import MapboxTokenError from "./MapboxTokenError" interface Props { @@ -97,12 +95,7 @@ const withMapboxToken = (deltaType: string) => ( // If our mapboxToken hasn't been retrieved yet, show a loading alert. if (isFetching) { - return ( - - ) + return } // We have the mapbox token. Pass it through to our component.