<Alert> 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.
streamlit · Nov 11, 2020 · a8b14f2 · a8b14f2
1 parent e7360df
commit a8b14f2
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 52 deletions.
diff --git 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<Props> {
           <ErrorBoundary width={width}>
             <Suspense
               fallback={
-                <Alert
-                  element={
-                    makeElementWithInfoText("Loading...").alert as AlertProto
-                  }
-                  width={width}
-                />
+                <Alert body="Loading..." kind={Kind.INFO} width={width} />
               }
             >
               {element}
@@ -347,10 +344,16 @@ class Block extends PureComponent<Props> {
     }
 
     switch (node.element.type) {
-      case "alert":
+      case "alert": {
+        const alertProto = node.element.alert as AlertProto
         return (
-          <Alert width={width} element={node.element.alert as AlertProto} />
+          <Alert
+            width={width}
+            body={alertProto.body}
+            kind={getAlertKind(alertProto.format)}
+          />
         )
+      }
 
       case "audio":
         return (

diff --git 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<AlertProto> = {}): AlertProps => ({
-  element: AlertProto.create({
-    body: "Something happened!",
-    ...elementProps,
-  }),
+const getProps = (elementProps: Partial<AlertProps> = {}): 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(<Alert {...props} />)
@@ -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(<Alert {...props} />)
@@ -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(<Alert {...props} />)
@@ -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(<Alert {...props} />)
@@ -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(<Alert {...props} />)
-    }).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
@@ -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 (
     <div className="stAlert">
-      <AlertContainer width={width} kind={getAlertKind(format)}>
+      <AlertContainer width={width} kind={kind}>
         <div className="markdown-text-container">
           <StreamlitMarkdown source={body} allowHTML={false} />
         </div>

diff --git 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 (
-          <Alert
-            element={makeElementWithInfoText("Loading...").alert as AlertProto}
-            width={width}
-          />
-        )
+        return <Alert body={"Loading..."} kind={Kind.INFO} width={width} />
       }
 
       // We have the mapbox token. Pass it through to our component.