Make ribbon block error indicators more obvious

webview/src/plots/components/App.test.tsx

@@ -49,6 +49,7 @@ import {
   toggleDragAndDropMode as toggleCustomPlotsDragAndDropMode
 } from './customPlots/customPlotsSlice'
 import { comparisonTableInitialState } from './comparisonTable/comparisonTableSlice'
+import plotRibbonStyles from './ribbon/styles.module.scss'
 import { plotsReducers, plotsStore } from '../store'
 import { vsCodeApi } from '../../shared/api'
 import {
@@ -2763,11 +2764,11 @@ describe('App', () => {
       })
     })
 
-    it('should show an error indicator for each revision with an error', () => {
+    it('should show an error icon and highlight the title for each revision with an error', () => {
       renderAppWithOptionalData({
         comparison: comparisonTableFixture,
         selectedRevisions: plotsRevisionsFixture.map(rev => {
-          if (rev.label === 'main') {
+          if (rev.label === 'main' || rev.description === '[exp-e7a67]') {
             return {
               ...rev,
               errors: ['error']
@@ -2776,11 +2777,19 @@ describe('App', () => {
           return rev
         })
       })
-      const errorIndicators = screen.getAllByText('!')
-      expect(errorIndicators).toHaveLength(1)
+      const mainTitle = within(screen.getByTestId('ribbon-main')).getByText(
+        'main'
+      )
+      const expTitle = within(screen.getByTestId('ribbon-exp-e7a67')).getByText(
+        '[exp-e7a67]'
+      )
+      const errorIndicators = screen.getAllByLabelText('Error Icon')
+      expect(errorIndicators).toHaveLength(2)
+      expect(mainTitle).toHaveClass(plotRibbonStyles.errorIndicator)
+      expect(expTitle).toHaveClass(plotRibbonStyles.errorIndicator)
     })
 
-    it('should not show an error indicator for a loading revision', () => {
+    it('should not show an error icon or highlight the title for a loading revision', () => {
       renderAppWithOptionalData({
         comparison: comparisonTableFixture,
         selectedRevisions: plotsRevisionsFixture.map(rev => {
@@ -2794,7 +2803,11 @@ describe('App', () => {
           return rev
         })
       })
-      expect(screen.queryByText('!')).not.toBeInTheDocument()
+      expect(screen.queryByLabelText('Error Icon')).not.toBeInTheDocument()
+      const titleText = within(screen.getByTestId('ribbon-main')).getByText(
+        'main'
+      )
+      expect(titleText).not.toHaveClass(plotRibbonStyles.errorIndicator)
     })
   })
 

webview/src/plots/components/ribbon/RevisionIcon.tsx

@@ -5,10 +5,8 @@ import styles from './styles.module.scss'
 
 export const RevisionIcon: React.FC<{
   fetched: boolean
-  errors?: string[]
-}> = ({ fetched, errors }) => (
+}> = ({ fetched }) => (
   <div className={styles.iconPlaceholder}>
-    {fetched && errors && '!'}
     {!fetched && (
       <VSCodeProgressRing className={cx(styles.fetching, 'chromatic-ignore')} />
     )}

webview/src/plots/components/ribbon/RibbonBlock.tsx

@@ -1,12 +1,14 @@
 import { Revision } from 'dvc/src/plots/webview/contract'
+import cx from 'classnames'
 import React from 'react'
 import styles from './styles.module.scss'
+import { RibbonBlockIcon } from './RibbonBlockIcon'
 import { RibbonBlockTooltip } from './RibbonBlockTooltip'
 import { RevisionIcon } from './RevisionIcon'
 import { Icon } from '../../../shared/components/Icon'
 import Tooltip from '../../../shared/components/tooltip/Tooltip'
 import { CopyButton } from '../../../shared/components/copyButton/CopyButton'
-import { Close, Info } from '../../../shared/components/icons'
+import { Close } from '../../../shared/components/icons'
 
 interface RibbonBlockProps {
   revision: Revision
@@ -27,19 +29,22 @@ export const RibbonBlock: React.FC<RibbonBlockProps> = ({
     id,
     label
   } = revision
+  const hasError = fetched && !!errors
 
   const mainContent = (
     <li
       className={styles.block}
       style={{ borderColor: displayColor }}
       data-testid={`ribbon-${id}`}
     >
-      <Info width={14} height={14} className={styles.infoIcon} />
+      <RibbonBlockIcon hasError={hasError} />
       <div className={styles.label}>
         {description ? (
           <>
             <div className={styles.subtitle}>{label}</div>
-            <div className={styles.title}>
+            <div
+              className={cx(styles.title, hasError && styles.errorIndicator)}
+            >
               {description}
               <CopyButton
                 value={description.replace(/[[\]]/g, '')}
@@ -48,14 +53,14 @@ export const RibbonBlock: React.FC<RibbonBlockProps> = ({
             </div>
           </>
         ) : (
-          <div className={styles.title}>
+          <div className={cx(styles.title, hasError && styles.errorIndicator)}>
             {label}
             <CopyButton value={label} className={styles.copyButton} />
           </div>
         )}
       </div>
       <div className={styles.iconPlaceholder}>
-        <RevisionIcon errors={errors} fetched={fetched} />
+        <RevisionIcon fetched={fetched} />
       </div>
       <Tooltip content="Clear" placement="bottom" delay={500}>
         <button className={styles.clearButton} onClick={onClear}>

webview/src/plots/components/ribbon/RibbonBlockIcon.tsx

@@ -0,0 +1,23 @@
+import React from 'react'
+import cx from 'classnames'
+import styles from './styles.module.scss'
+import { Error, Info } from '../../../shared/components/icons'
+
+export const RibbonBlockIcon: React.FC<{
+  hasError: boolean
+}> = ({ hasError }) =>
+  hasError ? (
+    <Error
+      width={14}
+      height={14}
+      className={cx(styles.blockIcon, styles.error)}
+      aria-label="Error Icon"
+    />
+  ) : (
+    <Info
+      width={14}
+      height={14}
+      className={styles.blockIcon}
+      aria-label="Info Icon"
+    />
+  )

webview/src/plots/components/ribbon/styles.module.scss

@@ -1,11 +1,16 @@
 @import '../../../shared/variables';
 @import '../../../shared/mixins';
 
+.errorIndicator {
+  color: $error-color;
+}
+
 .copyButton {
   display: none;
   position: absolute;
   right: 0;
   top: 0;
+  fill: $fg-color;
 }
 
 .tooltipColumn {
@@ -96,11 +101,6 @@
   align-items: center;
   font-size: 0.8125rem;
 
-  svg {
-    fill: $fg-color;
-    display: block;
-  }
-
   &:hover .copyButton {
     display: inline;
     font-size: 0.8125rem;
@@ -110,18 +110,16 @@
     margin-left: -4px;
     height: 18px;
     width: 18px;
-    color: $error-color;
-    text-align: center;
   }
 
   .fetching {
     height: 18px;
     width: 18px;
   }
+}
 
-  .infoIcon {
-    margin-left: 5px;
-  }
+.blockIcon {
+  margin-left: 5px;
 }
 
 .label {
@@ -153,6 +151,7 @@
 
 .clearButton {
   background-color: transparent;
+  color: $fg-color;
   border: none;
   padding: 3px;
   opacity: 0;