Use Alert component for track messages (#3200)

* Use Alert component for track messages * T1 * Possible BlockMsg update type * Possible BlockMsg update type * Update snaps * Update snaps again Co-authored-by: Colin <[email protected]>
GMOD · Sep 30, 2022 · 8b2dfbe · 8b2dfbe
1 parent e6c8ca8
commit 8b2dfbe
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 135 deletions.
diff --git a/plugins/linear-genome-view/src/BaseLinearDisplay/components/BlockMsg.tsx b/plugins/linear-genome-view/src/BaseLinearDisplay/components/BlockMsg.tsx
@@ -0,0 +1,42 @@
+import React from 'react'
+import { Tooltip, Button, Alert, AlertColor } from '@mui/material'
+import { makeStyles } from 'tss-react/mui'
+
+const useStyles = makeStyles()({
+  ellipses: {
+    textOverflow: 'ellipsis',
+    overflow: 'hidden',
+  },
+})
+
+export default function BlockMsg({
+  message,
+  severity,
+  buttonText,
+  icon,
+  action,
+}: {
+  message: string
+  severity?: AlertColor
+  buttonText?: string
+  icon?: React.ReactNode
+  action?: () => void
+}) {
+  const { classes } = useStyles()
+  const button = action ? (
+    <Button data-testid="reload_button" onClick={action} startIcon={icon}>
+      {buttonText}
+    </Button>
+  ) : null
+  return (
+    <Tooltip title={message}>
+      <Alert
+        severity={severity}
+        action={button}
+        classes={{ message: classes.ellipses }}
+      >
+        {message}
+      </Alert>
+    </Tooltip>
+  )
+}
diff --git a/...ns/linear-genome-view/src/BaseLinearDisplay/components/ServerSideRenderedBlockContent.tsx b/...ns/linear-genome-view/src/BaseLinearDisplay/components/ServerSideRenderedBlockContent.tsx
@@ -1,11 +1,12 @@
 import React, { useEffect, useState } from 'react'
-import { Typography, Button } from '@mui/material'
+import { Typography } from '@mui/material'
 import { makeStyles } from 'tss-react/mui'
 import { observer } from 'mobx-react'
 import { getParent } from 'mobx-state-tree'
-import { getParentRenderProps } from '@jbrowse/core/util/tracks'
 import RefreshIcon from '@mui/icons-material/Refresh'
 
+import BlockMsg from './BlockMsg'
+
 const useStyles = makeStyles()(theme => ({
   loading: {
     paddingLeft: '0.6em',
@@ -17,26 +18,6 @@ const useStyles = makeStyles()(theme => ({
     pointerEvents: 'none',
     textAlign: 'center',
   },
-  blockMessage: {
-    width: '100%',
-    background: theme.palette.action.disabledBackground,
-    padding: theme.spacing(2),
-    pointerEvents: 'none',
-    textAlign: 'center',
-  },
-  blockError: {
-    padding: theme.spacing(2),
-    width: '100%',
-    whiteSpace: 'normal',
-    color: theme.palette.error.main,
-    overflowY: 'auto',
-  },
-  blockReactNodeMessage: {
-    width: '100%',
-    background: theme.palette.action.disabledBackground,
-    padding: theme.spacing(2),
-    textAlign: 'center',
-  },
   dots: {
     '&::after': {
       display: 'inline-block',
@@ -59,15 +40,6 @@ const useStyles = makeStyles()(theme => ({
   },
 }))
 
-function Repeater({ children }: { children: React.ReactNode }) {
-  return (
-    <div style={{ display: 'flex' }}>
-      {children}
-      {children}
-    </div>
-  )
-}
-
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const LoadingMessage = observer(({ model }: { model: any }) => {
   // only show the loading message after 300ms to prevent excessive flickering
@@ -103,50 +75,6 @@ const LoadingMessage = observer(({ model }: { model: any }) => {
   )
 })
 
-function BlockMessage({
-  messageContent,
-}: {
-  messageContent: string | React.ReactNode
-}) {
-  const { classes } = useStyles()
-
-  return React.isValidElement(messageContent) ? (
-    <div className={classes.blockReactNodeMessage}>{messageContent}</div>
-  ) : (
-    <Typography variant="body2" className={classes.blockMessage}>
-      {messageContent}
-    </Typography>
-  )
-}
-
-function BlockError({
-  error,
-  reload,
-  displayHeight,
-}: {
-  error: Error
-  reload: () => void
-  displayHeight: number
-}) {
-  const { classes } = useStyles()
-  return (
-    <div className={classes.blockError} style={{ height: displayHeight }}>
-      {reload ? (
-        <Button
-          data-testid="reload_button"
-          onClick={reload}
-          startIcon={<RefreshIcon />}
-        >
-          Reload
-        </Button>
-      ) : null}
-      <Typography color="error" variant="body2" display="inline">
-        {`${error}`}
-      </Typography>
-    </div>
-  )
-}
-
 const ServerSideRenderedBlockContent = observer(
   ({
     model,
@@ -156,28 +84,25 @@ const ServerSideRenderedBlockContent = observer(
   }) => {
     if (model.error) {
       return (
-        <Repeater>
-          <BlockError
-            error={model.error}
-            reload={model.reload}
-            displayHeight={getParentRenderProps(model).displayModel.height}
-          />
-        </Repeater>
+        <BlockMsg
+          message={`${model.error}`}
+          severity="error"
+          buttonText="reload"
+          icon={<RefreshIcon />}
+          action={model.reload}
+        />
       )
     }
     if (model.message) {
-      return (
-        <Repeater>
-          <BlockMessage messageContent={model.message} />
-        </Repeater>
+      // the message can be a fully rendered react component, e.g. the region too large message
+      return React.isValidElement(model.message) ? (
+        model.message
+      ) : (
+        <BlockMsg message={`${model.message}`} severity="info" />
       )
     }
     if (!model.filled) {
-      return (
-        <Repeater>
-          <LoadingMessage model={model} />
-        </Repeater>
-      )
+      return <LoadingMessage model={model} />
     }
     return model.reactElement
   },

diff --git a/plugins/linear-genome-view/src/BaseLinearDisplay/models/BaseLinearDisplayModel.tsx b/plugins/linear-genome-view/src/BaseLinearDisplay/models/BaseLinearDisplayModel.tsx
@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import React from 'react'
-import { Button, Typography } from '@mui/material'
+import TooLargeMessage from './TooLargeMessage'
 import { BaseDisplay } from '@jbrowse/core/pluggableElementTypes/models'
 import { getConf } from '@jbrowse/core/configuration'
 import { MenuItem } from '@jbrowse/core/ui'
@@ -498,36 +498,8 @@ export const BaseLinearDisplay = types
      *  react node allows user to force load at current setting
      */
     regionCannotBeRendered(_region: Region) {
-      const { regionTooLarge, regionTooLargeReason } = self
-
-      if (regionTooLarge) {
-        return (
-          <>
-            <Typography component="span" variant="body2">
-              {regionTooLargeReason ? regionTooLargeReason + '. ' : ''}
-              Zoom in to see features or{' '}
-            </Typography>
-            <Button
-              data-testid="force_reload_button"
-              onClick={() => {
-                if (!self.estimatedRegionStats) {
-                  console.error('No global stats?')
-                } else {
-                  self.updateStatsLimit(self.estimatedRegionStats)
-                  self.reload()
-                }
-              }}
-              variant="outlined"
-            >
-              Force Load
-            </Button>
-            <Typography component="span" variant="body2">
-              (force load may be slow)
-            </Typography>
-          </>
-        )
-      }
-      return undefined
+      const { regionTooLarge } = self
+      return regionTooLarge ? <TooLargeMessage model={self} /> : null
     },
 
     trackMenuItems(): MenuItem[] {

diff --git a/plugins/linear-genome-view/src/BaseLinearDisplay/models/TooLargeMessage.tsx b/plugins/linear-genome-view/src/BaseLinearDisplay/models/TooLargeMessage.tsx
@@ -0,0 +1,34 @@
+import React from 'react'
+import BlockMsg from '../components/BlockMsg'
+import { Stats } from '@jbrowse/core/data_adapters/BaseAdapter'
+
+function TooLargeMessage({
+  model,
+}: {
+  model: {
+    regionTooLargeReason: string
+    estimatedRegionStats?: Stats
+    updateStatsLimit: (s: Stats) => void
+    reload: () => void
+  }
+}) {
+  const { regionTooLargeReason } = model
+  return (
+    <BlockMsg
+      severity="warning"
+      action={() => {
+        if (!model.estimatedRegionStats) {
+          console.error('No global stats?')
+        } else {
+          model.updateStatsLimit(model.estimatedRegionStats)
+          model.reload()
+        }
+      }}
+      buttonText="Force load"
+      message={`${regionTooLargeReason ? regionTooLargeReason + '. ' : ''}
+      Zoom in to see features or force load (may be slow).`}
+    />
+  )
+}
+
+export default TooLargeMessage
diff --git a/...r-genome-view/src/LinearGenomeView/components/__snapshots__/LinearGenomeView.test.js.snap b/...r-genome-view/src/LinearGenomeView/components/__snapshots__/LinearGenomeView.test.js.snap
@@ -522,11 +522,7 @@ exports[`<LinearGenomeView /> renders one track, one region 1`] = `
               <div
                 class="tss-1dmfa10-contentBlock"
                 style="width: 100px;"
-              >
-                <div
-                  style="display: flex;"
-                />
-              </div>
+              />
               <div
                 class="tss-a9ifge-boundaryPaddingBlock"
                 style="background: none; width: 800px;"

diff --git a/products/jbrowse-web/src/tests/ErrorConditions.test.tsx b/products/jbrowse-web/src/tests/ErrorConditions.test.tsx
@@ -16,12 +16,7 @@ test('404 sequence file', async () => {
 })
 
 test('wrong assembly', async () => {
-  console.error = jest.fn()
   const { view, findAllByText } = createView()
   view.showTrack('volvox_wrong_assembly')
-  await findAllByText(
-    'Error: region assembly (volvox) does not match track assemblies (wombat)',
-    {},
-    delay,
-  )
+  await findAllByText(/does not match/, {}, delay)
 }, 15000)
diff --git a/products/jbrowse-web/src/tests/StatsEstimation.test.tsx b/products/jbrowse-web/src/tests/StatsEstimation.test.tsx
@@ -49,7 +49,7 @@ test('test stats estimation pileup, force load to see', async () => {
   )
 
   await findAllByText(/Requested too much data/, {}, delay)
-  const buttons = await findAllByText(/Force Load/, {}, delay)
+  const buttons = await findAllByText(/Force load/, {}, delay)
   fireEvent.click(buttons[0])
 
   expectCanvasMatch(
@@ -84,7 +84,7 @@ test('test stats estimation on vcf track, force load to see', async () => {
   fireEvent.click(await findByTestId('htsTrackEntry-variant_colors', {}, delay))
 
   await findAllByText(/Zoom in to see features/, {}, delay)
-  const buttons = await findAllByText(/Force Load/, {}, delay)
+  const buttons = await findAllByText(/Force load/, {}, delay)
   fireEvent.click(buttons[0])
   await findByTestId('box-test-vcf-605223', {}, delay)
 }, 30000)