From 4128407e1a1883e9d8ebb85e04085cd1a39d986a Mon Sep 17 00:00:00 2001
From: cchaos <caroline.horn@elastic.co>
Date: Fri, 29 May 2020 16:57:27 -0400
Subject: [PATCH] Review updates

---
 src-docs/src/views/expression/columns.js    | 194 ++++++++++----------
 src-docs/src/views/expression/stringing.tsx |   3 +
 src/components/expression/_expression.scss  |  58 +++---
 src/components/expression/expression.tsx    |  49 ++---
 4 files changed, 152 insertions(+), 152 deletions(-)

diff --git a/src-docs/src/views/expression/columns.js b/src-docs/src/views/expression/columns.js
index b20719b50bd..7d072c8c80a 100644
--- a/src-docs/src/views/expression/columns.js
+++ b/src-docs/src/views/expression/columns.js
@@ -2,8 +2,6 @@ import React, { useState } from 'react';
 
 import {
   EuiPopoverTitle,
-  EuiFlexItem,
-  EuiFlexGroup,
   EuiPopover,
   EuiSelect,
   EuiComboBox,
@@ -103,7 +101,7 @@ export default () => {
   };
 
   const renderPopover1 = () => (
-    <div>
+    <div style={{ width: 300 }}>
       <EuiPopoverTitle>INDICES</EuiPopoverTitle>
       <EuiComboBox
         placeholder="Select one or more indices"
@@ -118,109 +116,103 @@ export default () => {
   );
 
   const renderPopover2 = () => (
-    <div>
+    <div style={{ width: 150 }}>
       <EuiPopoverTitle>WHEN</EuiPopoverTitle>
-      <EuiFlexGroup gutterSize="s">
-        <EuiFlexItem grow={false} style={{ width: 150 }}>
-          <EuiSelect
-            compressed
-            value={example2.value}
-            onChange={changeExample2}
-            options={[
-              {
-                value: 'count()',
-                text: 'count()',
-              },
-              {
-                value: 'sum()',
-                text: 'sum()',
-              },
-              {
-                value: 'min()',
-                text: 'min()',
-              },
-              { value: 'max()', text: 'max()' },
-            ]}
-          />
-        </EuiFlexItem>
-      </EuiFlexGroup>
+      <EuiSelect
+        compressed
+        value={example2.value}
+        onChange={changeExample2}
+        options={[
+          {
+            value: 'count()',
+            text: 'count()',
+          },
+          {
+            value: 'sum()',
+            text: 'sum()',
+          },
+          {
+            value: 'min()',
+            text: 'min()',
+          },
+          { value: 'max()', text: 'max()' },
+        ]}
+      />
     </div>
   );
 
   return (
-    <EuiFlexGroup>
-      <EuiFlexItem style={{ maxWidth: 500 }}>
-        <EuiPopover
-          id="popover1"
-          button={
-            <EuiExpression
-              description="indices"
-              display="columns"
-              value={example1.value}
-              isInvalid={
-                selectedOptions && selectedOptions.length > 0 ? false : true
-              }
-              isActive={example1.isOpen}
-              onClick={openExample1}
-            />
-          }
-          zIndex={200}
-          isOpen={example1.isOpen}
-          closePopover={closeExample1}
-          ownFocus
-          panelPaddingSize="s"
-          anchorPosition="downLeft">
-          {renderPopover1()}
-        </EuiPopover>
+    <div style={{ maxWidth: 500 }}>
+      <EuiPopover
+        id="popover1"
+        button={
+          <EuiExpression
+            description="indices"
+            display="columns"
+            value={example1.value}
+            isInvalid={
+              selectedOptions && selectedOptions.length > 0 ? false : true
+            }
+            isActive={example1.isOpen}
+            onClick={openExample1}
+          />
+        }
+        isOpen={example1.isOpen}
+        closePopover={closeExample1}
+        ownFocus
+        display="block"
+        panelPaddingSize="s"
+        anchorPosition="downLeft">
+        {renderPopover1()}
+      </EuiPopover>
 
-        <EuiPopover
-          id="popover2"
-          panelPaddingSize="s"
-          zIndex={200}
-          button={
-            <EuiExpression
-              description="when"
-              display="columns"
-              value={example2.value}
-              isActive={example2.isOpen}
-              onClick={openExample2}
-            />
-          }
-          isOpen={example2.isOpen}
-          closePopover={closeExample2}
-          ownFocus
-          anchorPosition="downLeft">
-          {renderPopover2()}
-        </EuiPopover>
-        <EuiExpression
-          display="columns"
-          description="Except"
-          value="kibana_sample_data_ky_counties"
-        />
-        <EuiSpacer />
-        <EuiTitle size="xxs">
-          <h3>Description width at 50%</h3>
-        </EuiTitle>
-        <EuiExpression
-          description="join"
-          display="columns"
-          descriptionWidth={50}
-          value="kibana_sample_data_ky_avl"
-          onClick={() => {}}
-        />
-        <EuiSpacer />
-        <EuiTitle size="xxs">
-          <h3>Error state</h3>
-        </EuiTitle>
-        <EuiExpression
-          description="email"
-          display="columns"
-          color="danger"
-          isInvalid
-          value="example@mail."
-          onClick={() => {}}
-        />
-      </EuiFlexItem>
-    </EuiFlexGroup>
+      <EuiPopover
+        id="popover2"
+        panelPaddingSize="s"
+        button={
+          <EuiExpression
+            description="when"
+            display="columns"
+            value={example2.value}
+            isActive={example2.isOpen}
+            onClick={openExample2}
+          />
+        }
+        isOpen={example2.isOpen}
+        closePopover={closeExample2}
+        ownFocus
+        display="block"
+        anchorPosition="downLeft">
+        {renderPopover2()}
+      </EuiPopover>
+      <EuiExpression
+        display="columns"
+        description="Except"
+        value="kibana_sample_data_ky_counties"
+      />
+      <EuiSpacer />
+      <EuiTitle size="xxs">
+        <h3>Description width at 50px</h3>
+      </EuiTitle>
+      <EuiExpression
+        description="join"
+        display="columns"
+        descriptionWidth={50}
+        value="kibana_sample_data_ky_avl"
+        onClick={() => {}}
+      />
+      <EuiSpacer />
+      <EuiTitle size="xxs">
+        <h3>Error state</h3>
+      </EuiTitle>
+      <EuiExpression
+        description="email"
+        display="columns"
+        color="danger"
+        isInvalid
+        value="example@mail."
+        onClick={() => {}}
+      />
+    </div>
   );
 };
diff --git a/src-docs/src/views/expression/stringing.tsx b/src-docs/src/views/expression/stringing.tsx
index f8cefa571bf..0720edbae6e 100644
--- a/src-docs/src/views/expression/stringing.tsx
+++ b/src-docs/src/views/expression/stringing.tsx
@@ -24,6 +24,9 @@ export default () => (
       description="group by"
       value="right.kytccountynmbr"
       onClick={() => {}}
+      color="accent"
+      isInvalid
     />
+    <EuiExpression description="sort by" value="count" isInvalid />
   </div>
 );
diff --git a/src/components/expression/_expression.scss b/src/components/expression/_expression.scss
index df2e725a91a..166773c5738 100644
--- a/src/components/expression/_expression.scss
+++ b/src/components/expression/_expression.scss
@@ -7,11 +7,11 @@
   @include euiFontSizeS;
   @include euiCodeFont;
 
-  border-bottom: 2px solid transparent;
+  border-bottom: $euiBorderWidthThick solid transparent;
   display: inline-block; /* 1 */
   text-align: left;
   padding: ($euiSizeXS / 2) 0;
-  transition: all $euiAnimSpeedNormal $euiAnimSlightBounce;
+  transition: all $euiAnimSpeedNormal ease-in-out;
   color: $euiTextColor;
 
   &:focus {
@@ -23,10 +23,11 @@
   }
 
   &.euiExpression-columns {
-    border: 2px solid transparent;
+    border-color: transparent;
+    // Ensures there's no flash of the dashed style before turning solid for the active state
+    border-bottom-style: solid;
     margin-bottom: $euiSizeXS;
   }
-
 }
 
 .euiExpression-isUppercase .euiExpression__description {
@@ -41,54 +42,54 @@
     border-bottom-style: solid;
     transform: translateY(-1px);
   }
+}
+
+.euiExpression__icon {
+  margin-left: $euiSizeXS;
+}
 
+.euiExpression-isActive {
+  border-bottom-style: solid;
 }
 
 .euiExpression-columns {
-  background-color: $euiColorLightestShade;
   width: 100%;
   display: flex;
-  padding: $euiSizeXS * .5;
+  padding: $euiSizeXS;
   border-radius: $euiSizeXS;
 
-  &:not(.euiExpression-isClickable) {
-    .euiExpression__description {
-      color: $euiTextColor;
+  &.euiExpression-isClickable {
+    background-color: $euiColorLightestShade;
+
+    // sass-lint:disable-block nesting-depth
+    &:focus,
+    &:hover:not(:disabled) {
+      .euiExpression__description,
+      .euiExpression__value {
+        // inner child specificity so it inherits underline color from text color
+        text-decoration: underline;
+      }
     }
   }
 
-  .euiExpression__description,
-  .euiExpression__value {
-    @include euiTextBreakWord;
-  }
-
   .euiExpression__description {
-    margin: $euiSizeXS;
     text-align: right;
+    margin-right: $euiSizeS;
+    flex-shrink: 0; // Ensures it doesn't get smaller in case the value is really long
   }
 
   .euiExpression__value {
     flex-grow: 1;
-    display: flex;
-    margin: $euiSizeXS;
   }
 
   .euiExpression__icon {
-    margin-left: auto;
+    margin-top: $euiSizeXS;
   }
 }
 
-.euiExpression-isActive {
-  border-bottom-style: solid;
-}
-
-.euiPopover__anchor {
-  width: 100%;
-}
-
 @each $name, $color in $euiExpressionColors {
   .euiExpression--#{$name} {
-    &:focus:not(.euiExpression-columns) {
+    &:focus {
       background-color: transparentize($color, .9);
     }
 
@@ -100,6 +101,5 @@
     .euiExpression__description {
       color: $color;
     }
-
   }
-}
\ No newline at end of file
+}
diff --git a/src/components/expression/expression.tsx b/src/components/expression/expression.tsx
index 918191fea08..b540ede6009 100644
--- a/src/components/expression/expression.tsx
+++ b/src/components/expression/expression.tsx
@@ -19,7 +19,6 @@
 
 import React, {
   ButtonHTMLAttributes,
-  Fragment,
   HTMLAttributes,
   MouseEventHandler,
   ReactNode,
@@ -77,11 +76,17 @@ export type EuiExpressionProps = CommonProps & {
    * Sets the display style for the expression. Defaults to `inline`
    */
   display?: keyof typeof displayToClassNameMap;
+  /**
+   * Forces color to display as `danger` and shows an `alert` icon
+   */
   isInvalid?: boolean;
   /**
-   * Sets a custom width for the description when using the columns layout. Defaults to 30%
+   * Sets a custom width for the description when using the columns layout.
+   * Set to a number for a custom width in `px`.
+   * Set to a string for a custom width in custom measurement.
+   * Defaults to `20%`
    */
-  descriptionWidth?: number;
+  descriptionWidth?: number | string;
 };
 
 type Buttonlike = EuiExpressionProps &
@@ -103,17 +108,13 @@ export const EuiExpression: React.FunctionComponent<
   uppercase = true,
   isActive = false,
   display = 'inline',
-  descriptionWidth = 20,
+  descriptionWidth = '20%',
   onClick,
   isInvalid = false,
   ...rest
 }) => {
-  let colorClass;
-  if (display === 'columns' && isInvalid) {
-    colorClass = colorToClassNameMap.danger;
-  } else {
-    colorClass = colorToClassNameMap[color];
-  }
+  const calculatedColor = isInvalid ? 'danger' : color;
+
   const classes = classNames(
     'euiExpression',
     className,
@@ -123,18 +124,30 @@ export const EuiExpression: React.FunctionComponent<
       'euiExpression-isUppercase': uppercase,
     },
     displayToClassNameMap[display],
-    colorClass
+    colorToClassNameMap[calculatedColor]
   );
 
   const Component = onClick ? 'button' : 'span';
 
+  const descriptionStyle = descriptionProps && descriptionProps.style;
   const customWidth =
     display === 'columns' && descriptionWidth
       ? {
-          flexBasis: `${descriptionWidth}%`,
+          flexBasis: descriptionWidth,
+          ...descriptionStyle,
         }
       : undefined;
 
+  const invalidIcon = isInvalid ? (
+    <EuiIcon
+      className="euiExpression__icon"
+      type="alert"
+      color={calculatedColor}
+    />
+  ) : (
+    undefined
+  );
+
   return (
     <Component className={classes} onClick={onClick} {...rest}>
       <span
@@ -144,17 +157,9 @@ export const EuiExpression: React.FunctionComponent<
         {description}
       </span>{' '}
       <span className="euiExpression__value" {...valueProps}>
-        {display === 'columns' ? (
-          <Fragment>
-            <div>{value}</div>
-            <span className="euiExpression__icon">
-              {isInvalid ? <EuiIcon type="alert" color="danger" /> : null}
-            </span>
-          </Fragment>
-        ) : (
-          value
-        )}
+        {value}
       </span>
+      {invalidIcon}
     </Component>
   );
 };