From d3facd22be0ec59c31f59c9488c6397bc333ede4 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 10 Apr 2018 16:10:26 -0700 Subject: [PATCH 1/4] Wrap EuiHorizontalStep titles instead of truncating them. --- CHANGELOG.md | 5 ++ src-docs/src/views/steps/steps_horizontal.js | 5 +- .../steps_horizontal.test.js.snap | 61 ++++++++++--------- src/components/steps/_steps_horizontal.scss | 30 +++++---- src/components/steps/step_horizontal.js | 40 ++++++------ src/components/steps/steps_horizontal.test.js | 5 +- 6 files changed, 79 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca46f0bdeec..4c953503045 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ - Added support for `
` and `` tags to `` ([#654](https://github.com/elastic/eui/pull/654))
 - Added export of SASS theme variables in JSON format during compilation ([#642](https://github.com/elastic/eui/pull/642))
 - Close `EuiComboBox` `singleSelection` options list when option is choosen ([#645](https://github.com/elastic/eui/pull/645))
+- Wrap `EuiHorizontalStep` text instead of truncating it ([#653](https://github.com/elastic/eui/pull/653))
+
+**Breaking changes**
+
+- `EuiHorizontalSteps` now requires an `onClick` prop be provided for each step configuration object ([#653](https://github.com/elastic/eui/pull/653))
 
 ## [`0.0.40`](https://github.com/elastic/eui/tree/v0.0.40)
 
diff --git a/src-docs/src/views/steps/steps_horizontal.js b/src-docs/src/views/steps/steps_horizontal.js
index fd1bd43690f..bbdf9e27221 100644
--- a/src-docs/src/views/steps/steps_horizontal.js
+++ b/src-docs/src/views/steps/steps_horizontal.js
@@ -13,13 +13,16 @@ const horizontalSteps = [
   {
     title: 'Selected Step 2',
     isSelected: true,
+    onClick: () => window.alert('Step 2 clicked')
   },
   {
-    title: 'Incomplete Step 3',
+    title: 'Incomplete Step 3 which will wrap to the next line',
+    onClick: () => window.alert('Step 3 clicked')
   },
   {
     title: 'Disabled Step 4',
     disabled: true,
+    onClick: () => window.alert('Step 4 clicked')
   },
 ];
 
diff --git a/src/components/steps/__snapshots__/steps_horizontal.test.js.snap b/src/components/steps/__snapshots__/steps_horizontal.test.js.snap
index 6800cdfa80c..a77e8766871 100644
--- a/src/components/steps/__snapshots__/steps_horizontal.test.js.snap
+++ b/src/components/steps/__snapshots__/steps_horizontal.test.js.snap
@@ -7,18 +7,19 @@ exports[`EuiStepsHorizontal is rendered 1`] = `
   data-test-subj="test subject string"
   role="tablist"
 >
-  
-  
-  
-  
+    
+  
 
 `;
diff --git a/src/components/steps/_steps_horizontal.scss b/src/components/steps/_steps_horizontal.scss
index bc1f63d9c60..1d5639c45fc 100644
--- a/src/components/steps/_steps_horizontal.scss
+++ b/src/components/steps/_steps_horizontal.scss
@@ -4,8 +4,7 @@
 }
 
 /**
- * 1. Ensure the title truncates instead of wraps
- * 2. Ensure the connecting lines stays behind the number
+ * 1. Ensure the connecting lines stays behind the number
  */
 
 .euiStepsHorizontal {
@@ -20,11 +19,15 @@
   flex-grow: 1;
   flex-basis: 0%;
   padding: $euiSizeL $euiSize $euiSize;
-  overflow: hidden; /* 1 */
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+  justify-content: flex-start;
+  cursor: pointer;
 
   // focus & hover state
   &:focus,
-  &:hover:not(:disabled) {
+  &:hover:not(.euiStepHorizontal-isDisabled) {
     .euiStepHorizontal__number {
       background: $euiColorPrimary;
       color: $euiColorEmptyShade;
@@ -39,7 +42,7 @@
   }
 
   // disabled state
-  &[disabled] {
+  &.euiStepHorizontal-isDisabled {
     cursor: not-allowed;
   }
 
@@ -51,9 +54,9 @@
     position: absolute;
     width: 50%;
     height: 1px;
-    top: $euiSizeL + $euiStepNumberSize/2;
+    top: $euiSizeL + ($euiStepNumberSize / 2);
     background-color: $euiColorLightShade;
-    z-index: $euiZLevel0; /* 2 */
+    z-index: $euiZLevel0; /* 1 */
   }
 
   &::before {
@@ -76,8 +79,8 @@
 
 .euiStepHorizontal__number {
   @include createStepsNumber;
-  position: relative; /* 2 */
-  z-index: $euiZLevel1; /* 2 */
+  position: relative; /* 1 */
+  z-index: $euiZLevel1; /* 1 */
   transition: all $euiAnimSpeedFast ease-in-out;
 
   // if it contains an icon, it needs to shift up a couple px
@@ -88,17 +91,12 @@
 }
 
 .euiStepHorizontal__title {
-  display: block;
   @include euiTitle("xs");
   margin-top: $euiSizeS;
   font-weight: $euiFontWeightRegular;
+  text-align: center;
 
-  // truncate
-  white-space: nowrap;  /* 1 */
-  overflow: hidden;  /* 1 */
-  text-overflow: ellipsis;  /* 1 */
-
-  .euiStepHorizontal:disabled & {
+  .euiStepHorizontal-isDisabled & {
     color: $euiColorDarkShade;
   }
 }
diff --git a/src/components/steps/step_horizontal.js b/src/components/steps/step_horizontal.js
index 368cfaff34f..cacf6f5b1ea 100644
--- a/src/components/steps/step_horizontal.js
+++ b/src/components/steps/step_horizontal.js
@@ -4,6 +4,7 @@ import classNames from 'classnames';
 
 import {
   EuiScreenReaderOnly,
+  EuiKeyboardAccessible,
 } from '../accessibility';
 
 import { EuiIcon } from '../icon';
@@ -43,35 +44,34 @@ export const EuiStepHorizontal = ({
   const buttonTitle = `Step ${step}: ${title}${titleAppendix}`;
 
   return (
-    
+    
   );
 };
 
 EuiStepHorizontal.propTypes = {
   isSelected: PropTypes.bool,
   isComplete: PropTypes.bool,
-  onClick: PropTypes.func,
+  onClick: PropTypes.func.isRequired,
   step: PropTypes.number.isRequired,
   title: PropTypes.node,
   className: PropTypes.string,
diff --git a/src/components/steps/steps_horizontal.test.js b/src/components/steps/steps_horizontal.test.js
index bfae6aeee0a..aa6f12bedae 100644
--- a/src/components/steps/steps_horizontal.test.js
+++ b/src/components/steps/steps_horizontal.test.js
@@ -8,18 +8,21 @@ const steps = [
   {
     title: 'Completed Step 1',
     isComplete: true,
-    onClick: () => window.alert('Step 1 clicked'),
+    onClick: () => {},
   },
   {
     title: 'Selected Step 2',
     isSelected: true,
+    onClick: () => {},
   },
   {
     title: 'Incomplete Step 3',
+    onClick: () => {},
   },
   {
     title: 'Disabled Step 4',
     disabled: true,
+    onClick: () => {},
   },
 ];
 

From 1913c37373fcf9d33c9c8dbe647a5d5a9b5df577 Mon Sep 17 00:00:00 2001
From: CJ Cenizal 
Date: Wed, 11 Apr 2018 11:39:21 -0700
Subject: [PATCH 2/4] Disallow clicks on disabled steps.

---
 .../step_horizontal.test.js.snap              | 30 ++++++++++
 src/components/steps/step.test.js             |  9 ++-
 src/components/steps/step_horizontal.js       |  2 +-
 src/components/steps/step_horizontal.test.js  | 56 +++++++++++++++++++
 4 files changed, 91 insertions(+), 6 deletions(-)
 create mode 100644 src/components/steps/__snapshots__/step_horizontal.test.js.snap
 create mode 100644 src/components/steps/step_horizontal.test.js

diff --git a/src/components/steps/__snapshots__/step_horizontal.test.js.snap b/src/components/steps/__snapshots__/step_horizontal.test.js.snap
new file mode 100644
index 00000000000..216a9b4fc0d
--- /dev/null
+++ b/src/components/steps/__snapshots__/step_horizontal.test.js.snap
@@ -0,0 +1,30 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`EuiStepHorizontal is rendered 1`] = `
+
+`;
diff --git a/src/components/steps/step.test.js b/src/components/steps/step.test.js
index 3393d03e6eb..f1f89460a65 100644
--- a/src/components/steps/step.test.js
+++ b/src/components/steps/step.test.js
@@ -6,18 +6,17 @@ import { EuiStep } from './step';
 
 describe('EuiStep', () => {
   test('is rendered', () => {
-    const stepContent = (
Do this
);
     const component = render(
       
+      >
+        
Do this

+      
     );
 
-    expect(component)
-      .toMatchSnapshot();
+    expect(component).toMatchSnapshot();
   });
 });
diff --git a/src/components/steps/step_horizontal.js b/src/components/steps/step_horizontal.js
index cacf6f5b1ea..a2443cf7c74 100644
--- a/src/components/steps/step_horizontal.js
+++ b/src/components/steps/step_horizontal.js
@@ -50,7 +50,7 @@ export const EuiStepHorizontal = ({
         aria-selected={!!isSelected}
         aria-disabled={!!disabled}
         className={classes}
-        onClick={() => disabled ? onClick() : undefined}
+        onClick={() => !disabled ? onClick() : undefined}
         title={buttonTitle}
         {...rest}
       >
diff --git a/src/components/steps/step_horizontal.test.js b/src/components/steps/step_horizontal.test.js
new file mode 100644
index 00000000000..316180a457d
--- /dev/null
+++ b/src/components/steps/step_horizontal.test.js
@@ -0,0 +1,56 @@
+import React from 'react';
+import { render, mount } from 'enzyme';
+import sinon from 'sinon';
+import { requiredProps } from '../../test/required_props';
+
+import { EuiStepHorizontal } from './step_horizontal';
+
+describe('EuiStepHorizontal', () => {
+  test('is rendered', () => {
+    const component = render(
+       {}}
+      />
+    );
+
+    expect(component).toMatchSnapshot();
+  });
+
+  describe('props', () => {
+    describe('onClick', () => {
+      test('is called when clicked', () => {
+        const onClickHandler = sinon.stub();
+
+        const component = mount(
+          
+        );
+
+        component.simulate('click');
+
+        sinon.assert.calledOnce(onClickHandler);
+      });
+
+      test(`isn't called when clicked if it's disabled`, () => {
+        const onClickHandler = sinon.stub();
+
+        const component = mount(
+          
+        );
+
+        component.simulate('click');
+
+        sinon.assert.notCalled(onClickHandler);
+      });
+    });
+  });
+});

From 1b8c55ca0de79a03101706ccbb1a9ee4860d3f1e Mon Sep 17 00:00:00 2001
From: CJ Cenizal 
Date: Wed, 11 Apr 2018 12:01:21 -0700
Subject: [PATCH 3/4] Remove focusability from disabled steps.

---
 .../steps/__snapshots__/steps_horizontal.test.js.snap |  2 +-
 src/components/steps/_steps_horizontal.scss           |  2 +-
 src/components/steps/step_horizontal.js               | 11 ++++++++++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/components/steps/__snapshots__/steps_horizontal.test.js.snap b/src/components/steps/__snapshots__/steps_horizontal.test.js.snap
index a77e8766871..63ff92b666b 100644
--- a/src/components/steps/__snapshots__/steps_horizontal.test.js.snap
+++ b/src/components/steps/__snapshots__/steps_horizontal.test.js.snap
@@ -101,7 +101,7 @@ exports[`EuiStepsHorizontal is rendered 1`] = `
     aria-selected="false"
     class="euiStepHorizontal euiStepHorizontal-isIncomplete euiStepHorizontal-isDisabled"
     role="tab"
-    tabindex="0"
+    tabindex="-1"
     title="Step 4: Disabled Step 4 is disabled"
   >
     
 {
+    if (disabled) {
+      return;
+    }
+
+    onClick(e);
+  }
+
   const buttonTitle = `Step ${step}: ${title}${titleAppendix}`;
 
   return (
@@ -50,7 +58,8 @@ export const EuiStepHorizontal = ({
         aria-selected={!!isSelected}
         aria-disabled={!!disabled}
         className={classes}
-        onClick={() => !disabled ? onClick() : undefined}
+        onClick={onStepClick}
+        tabIndex={disabled ? '-1' : '0'}
         title={buttonTitle}
         {...rest}
       >

From 3753dc4157056d876e410e87494e1ee178b7d36c Mon Sep 17 00:00:00 2001
From: CJ Cenizal 
Date: Wed, 11 Apr 2018 12:02:00 -0700
Subject: [PATCH 4/4] Add comments to explain flexbox styles.

---
 src/components/steps/_steps_horizontal.scss | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/components/steps/_steps_horizontal.scss b/src/components/steps/_steps_horizontal.scss
index 7f6bbfda345..6536d5738a5 100644
--- a/src/components/steps/_steps_horizontal.scss
+++ b/src/components/steps/_steps_horizontal.scss
@@ -5,6 +5,9 @@
 
 /**
  * 1. Ensure the connecting lines stays behind the number
+ * 2. Make each step the same width
+ * 3. Make the content of each step align to the top, even if the steps are of varying heights,
+ *    e.g. due to some of their titles wrapping to multiple lines
  */
 
 .euiStepsHorizontal {
@@ -16,13 +19,13 @@
 
 // Button containing item
 .euiStepHorizontal {
-  flex-grow: 1;
-  flex-basis: 0%;
+  flex-grow: 1; /* 2 */
+  flex-basis: 0%; /* 2 */
   padding: $euiSizeL $euiSize $euiSize;
-  display: flex;
-  flex-direction: column;
-  align-items: center;
-  justify-content: flex-start;
+  display: flex; /* 3 */
+  flex-direction: column; /* 3 */
+  align-items: center; /* 3 */
+  justify-content: flex-start; /* 3 */
   cursor: pointer;
 
   // focus & hover state