address code comments

elastic · May 17, 2017 · 694c5bf · 694c5bf
1 parent 3bbd73b
commit 694c5bf
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 90 deletions.
diff --git a/src/core_plugins/kibana/public/dashboard/dashboard.js b/src/core_plugins/kibana/public/dashboard/dashboard.js
@@ -24,7 +24,6 @@ import { DashboardState } from './dashboard_state';
 import { notify } from 'ui/notify';
 import { documentationLinks } from 'ui/documentation_links/documentation_links';
 import { showCloneModal } from './top_nav/show_clone_modal';
-import { DashboardCloneModal } from './top_nav/clone_modal';
 
 const app = uiModules.get('app/dashboard', [
   'elasticsearch',
@@ -33,13 +32,8 @@ const app = uiModules.get('app/dashboard', [
   'kibana/config',
   'kibana/notify',
   'kibana/typeahead',
-  'react',
 ]);
 
-app.directive('dashboardCloneModal', function (reactDirective) {
-  return reactDirective(DashboardCloneModal);
-});
-
 uiRoutes
   .when(DashboardConstants.CREATE_NEW_DASHBOARD_URL, {
     template: dashboardTemplate,

diff --git a/src/core_plugins/kibana/public/dashboard/top_nav/__snapshots__/clone_modal.test.js.snap b/src/core_plugins/kibana/public/dashboard/top_nav/__snapshots__/clone_modal.test.js.snap
@@ -2,64 +2,68 @@
 
 exports[`renders DashboardCloneModal 1`] = `
 <div
-  aria-label="Clone a dashboard"
-  class="kuiModal dashboardCloneModal"
-  data-tests-subj="dashboardCloneModal"
+  class="kuiModalOverlay"
 >
   <div
-    class="kuiModalHeader"
+    aria-label="Clone a dashboard"
+    class="kuiModal dashboardCloneModal"
+    data-tests-subj="dashboardCloneModal"
   >
     <div
-      class="kuiModalHeader__title"
+      class="kuiModalHeader"
     >
-      Clone Dashboard
+      <div
+        class="kuiModalHeader__title"
+      >
+        Clone Dashboard
+      </div>
     </div>
-  </div>
-  <div
-    class="kuiModalBody"
-  >
     <div
-      class="kuiModalBodyText kuiVerticalRhythm"
+      class="kuiModalBody"
     >
-      Please enter a new name for your dashboard.
+      <div
+        class="kuiModalBodyText kuiVerticalRhythm"
+      >
+        Please enter a new name for your dashboard.
+      </div>
+      <div
+        class="kuiModalBodyText kuiVerticalRhythm"
+      >
+        <input
+          class="kuiTextInput kuiTextInput--large"
+          data-test-subj="clonedDashboardTitle"
+          value="dash title"
+        />
+      </div>
     </div>
     <div
-      class="kuiModalBodyText kuiVerticalRhythm"
+      class="kuiModalFooter"
     >
-      <input
-        class="kuiTextInput kuiTextInput--large"
-        data-test-subj="clonedDashboardTitle"
-        value="dash title"
-      />
-    </div>
-  </div>
-  <div
-    class="kuiModalFooter"
-  >
-    <button
-      class="kuiButton kuiButton--hollow"
-      data-test-subj="cloneCancelButton"
-    >
-      <span
-        class="kuiButton__inner"
+      <button
+        class="kuiButton kuiButton--hollow"
+        data-test-subj="cloneCancelButton"
       >
-        <span>
-          Cancel
+        <span
+          class="kuiButton__inner"
+        >
+          <span>
+            Cancel
+          </span>
         </span>
-      </span>
-    </button>
-    <button
-      class="kuiButton kuiButton--primary"
-      data-test-subj="cloneConfirmButton"
-    >
-      <span
-        class="kuiButton__inner"
+      </button>
+      <button
+        class="kuiButton kuiButton--primary"
+        data-test-subj="cloneConfirmButton"
       >
-        <span>
-          Confirm Clone
+        <span
+          class="kuiButton__inner"
+        >
+          <span>
+            Confirm Clone
+          </span>
         </span>
-      </span>
-    </button>
+      </button>
+    </div>
   </div>
 </div>
 `;
diff --git a/src/core_plugins/kibana/public/dashboard/top_nav/clone_modal.js b/src/core_plugins/kibana/public/dashboard/top_nav/clone_modal.js
@@ -21,10 +21,6 @@ export class DashboardCloneModal extends React.Component {
     };
   }
 
-  componentDidMount() {
-    this.confirmButton.focus();
-  }
-
   cloneDashboard = () => {
     this.props.onClone(this.state.newDashboardName);
   };
@@ -34,7 +30,7 @@ export class DashboardCloneModal extends React.Component {
   };
 
   onKeyDown = (event) => {
-    if (event.keyCode === 27) {
+    if (event.keyCode === 27) { // ESC key
       this.props.onClose();
     }
   };
@@ -59,6 +55,7 @@ export class DashboardCloneModal extends React.Component {
             </KuiModalBodyText>
             <KuiModalBodyText className="kuiVerticalRhythm">
               <input
+                autoFocus
                 data-test-subj="clonedDashboardTitle"
                 className="kuiTextInput kuiTextInput--large"
                 value={ this.state.newDashboardName }
@@ -78,7 +75,6 @@ export class DashboardCloneModal extends React.Component {
               type="primary"
               data-test-subj="cloneConfirmButton"
               onClick={ this.cloneDashboard }
-              ref={ (button) => { this.confirmButton = button; } }
             >
               Confirm Clone
             </KuiButton>

diff --git a/ui_framework/components/button/button.js b/ui_framework/components/button/button.js
@@ -59,42 +59,34 @@ const ContentWithIcon = ({ children, icon, iconPosition, isLoading }) => {
   }
 };
 
-class KuiButton extends React.Component {
-  focus() {
-    this.kuiButton.focus();
-  }
-
-  render() {
-    const {
-      isLoading,
-      iconPosition = DEFAULT_ICON_POSITION,
-      className,
-      type,
-      icon,
-      children,
-      ...rest
-    } = this.props;
-    return (
-      <button
-        ref={ (kuiButton) => { this.kuiButton = kuiButton; } }
-        className={getClassName({
-          className,
-          type,
-          hasIcon: icon || isLoading,
-        })}
-        {...rest}
+const KuiButton = ({
+  isLoading,
+  iconPosition = DEFAULT_ICON_POSITION,
+  className,
+  type,
+  icon,
+  children,
+  ...rest
+}) => {
+  return (
+    <button
+      className={getClassName({
+        className,
+        type,
+        hasIcon: icon || isLoading,
+      })}
+      {...rest}
+    >
+      <ContentWithIcon
+        icon={icon}
+        iconPosition={iconPosition}
+        isLoading={isLoading}
       >
-        <ContentWithIcon
-          icon={icon}
-          iconPosition={iconPosition}
-          isLoading={isLoading}
-        >
-          {children}
-        </ContentWithIcon>
-      </button>
-    );
-  }
-}
+        {children}
+      </ContentWithIcon>
+    </button>
+  );
+};
 
 KuiButton.propTypes = {
   icon: PropTypes.node,