Merge pull request #4802 from jyotisingh/issue_4775

Fixed issue of duplicate html elements showing up on new dashboard -#4775
gocd · Jun 6, 2018 · 58e85fc · 58e85fc
2 parents 3bcc849 + 89b7991
commit 58e85fc
Show file tree

Hide file tree

Showing 6 changed files with 170 additions and 56 deletions.
diff --git a/...r/webapp/WEB-INF/rails.new/app/assets/new_stylesheets/single_page_apps/new_dashboard.scss b/...r/webapp/WEB-INF/rails.new/app/assets/new_stylesheets/single_page_apps/new_dashboard.scss
@@ -653,17 +653,39 @@ body {
   word-break:  break-all;
 }
 
-.pipeline_sub_header {
-  width:           240px;
-  display:         flex;
-  justify-content: space-between;
+#dashboard {
+  .tooltip {
+    left:  -100%;
+    top:   90%;
+    width: 10rem;
+    &::before {
+      left: 35%;
+    }
+  }
+  .pipeline-group_title {
+    .tooltip {
+      top: 120%;
+    }
+  }
 
-  .pipeline_actions {
-    display:     flex;
-    position:    relative;
-    top:         -3px;
-    flex-shrink: 0;
-    right:       0;
+  .pipeline_sub_header {
+    width:           240px;
+    display:         flex;
+    justify-content: space-between;
+
+    .pipeline_actions {
+      display:     flex;
+      position:    relative;
+      top:         -3px;
+      flex-shrink: 0;
+      right:       0;
+      .tooltip {
+        transform: translate(-53%, 15%);
+        &::before {
+          left: 75%;
+        }
+      }
+    }
   }
 }
 

diff --git a/server/webapp/WEB-INF/rails.new/spec/webpack/lib/form_helper_spec.js b/server/webapp/WEB-INF/rails.new/spec/webpack/lib/form_helper_spec.js
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2018 ThoughtWorks, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+describe("Form Helper", () => {
+  const $      = require("jquery");
+  const m      = require('mithril');
+  const Stream = require('mithril/stream');
+  require('jasmine-jquery');
+  const f = require('helpers/form_helper');
+
+  let $root, root;
+  beforeEach(() => {
+    [$root, root] = window.createDomElementForTest();
+  });
+  afterEach(window.destroyDomElementForTest);
+  const unmount = () => {
+    m.mount(root, null);
+    m.redraw();
+  };
+
+  afterEach(() => {
+    unmount();
+  });
+
+  it("buttonWithTooltip should add a tooltip to the button", () => {
+    const model = Stream(true);
+    m.mount(root, {
+      view() {
+        return m(f.buttonWithTooltip, {
+          model,
+          'tooltipText': "show me on hover",
+        }, "Fancy button with tooltip");
+      }
+    });
+    m.redraw();
+    const buttonWithTooltip = $root.find('button');
+    expect(buttonWithTooltip).toExist();
+    const tooltipId = $(buttonWithTooltip).attr("data-tooltip-id");
+    const tooltip   = $root.find(`#${tooltipId}`);
+    expect(tooltip).toExist();
+    expect(tooltip).toHaveText("show me on hover");
+    expect(tooltip).not.toBeVisible();
+    $(buttonWithTooltip).trigger('mouseover');
+    expect(tooltip).toBeVisible();
+    $(buttonWithTooltip).trigger('mouseout');
+    expect(tooltip).not.toBeVisible();
+  });
+
+  it("linkWithTooltip should add a tooltip to the link", () => {
+    const model = Stream(true);
+    m.mount(root, {
+      view() {
+        return m(f.linkWithTooltip, {
+          model,
+          'tooltipText': "show me on hover",
+        }, "Fancy link with tooltip");
+      }
+    });
+    m.redraw();
+    const linkWithTooltip = $root.find('a');
+    expect(linkWithTooltip).toExist();
+    const tooltipId = $(linkWithTooltip).attr("data-tooltip-id");
+    const tooltip   = $root.find(`#${tooltipId}`);
+    expect(tooltip).toExist();
+    expect(tooltip).toHaveText("show me on hover");
+    expect(tooltip).not.toBeVisible();
+    $(linkWithTooltip).trigger('mouseover');
+    expect(tooltip).toBeVisible();
+    $(linkWithTooltip).trigger('mouseout');
+    expect(tooltip).not.toBeVisible();
+  });
+});
diff --git a/server/webapp/WEB-INF/rails.new/spec/webpack/views/dashboard/dashboard_widget_spec.js b/server/webapp/WEB-INF/rails.new/spec/webpack/views/dashboard/dashboard_widget_spec.js
@@ -343,7 +343,7 @@ describe("Dashboard Widget", () => {
     mount(false);
 
     expect($root.find('.pipeline-group_title a')).toHaveClass('disabled');
-    expect($root.find('.pipeline-group_title a')).toHaveAttr('data-tooltip');
+    expect($root.find('.pipeline-group_title a')).toHaveAttr('data-tooltip-id');
   });
 
   it("should show plain text pipeline group name without link for non admin users", () => {

diff --git a/server/webapp/WEB-INF/rails.new/spec/webpack/views/dashboard/pipeline_widget_spec.js b/server/webapp/WEB-INF/rails.new/spec/webpack/views/dashboard/pipeline_widget_spec.js
@@ -111,7 +111,7 @@ describe("Dashboard Pipeline Widget", () => {
 
       expect(pipeline.canAdminister).toBe(false);
       expect($root.find('.edit_config')).toHaveClass('disabled');
-      expect($root.find('.edit_config')).toHaveAttr('data-tooltip');
+      expect($root.find('.edit_config')).toHaveAttr('data-tooltip-id');
     });
 
     it('should disable pipeline settings for config repo pipelines', () => {
@@ -446,10 +446,10 @@ describe("Dashboard Pipeline Widget", () => {
         unmount();
         mount(false, true, pauseInfo, {}, false);
         const pauseButton = $root.find('.pause');
-        expect(pauseButton).toHaveAttr('title');
-        expect(pauseButton).toHaveAttr('data-tooltip');
+        expect(pauseButton).toHaveAttr('data-tooltip-id');
+        const tooltipId = $(pauseButton).attr('data-tooltip-id');
+        expect($(`#${tooltipId}`)).toHaveText("You do not have permission to pause the pipeline.");
       });
-
     });
 
     describe("Unlock", () => {
@@ -648,8 +648,9 @@ describe("Dashboard Pipeline Widget", () => {
         unmount();
         mount(false, true, {}, {}, true, false);
         const playButton = $root.find('.pipeline_operations .play');
-        expect(playButton).toHaveAttr('title');
-        expect(playButton).toHaveAttr('data-tooltip');
+        expect(playButton).toHaveAttr('data-tooltip-id');
+        const tooltipId = $(playButton).attr('data-tooltip-id');
+        expect($(`#${tooltipId}`)).toHaveText("You do not have permission to trigger the pipeline");
       });
     });
 
@@ -839,8 +840,9 @@ describe("Dashboard Pipeline Widget", () => {
         unmount();
         mount(false, true, {}, {}, true, false);
         const playButton = $root.find('.pipeline_operations .play_with_options');
-        expect(playButton).toHaveAttr('title');
-        expect(playButton).toHaveAttr('data-tooltip');
+        expect(playButton).toHaveAttr('data-tooltip-id');
+        const tooltipId = $(playButton).attr('data-tooltip-id');
+        expect($(`#${tooltipId}`)).toHaveText("You do not have permission to trigger the pipeline");
       });
     });
   });

diff --git a/...webapp/WEB-INF/rails.new/spec/webpack/views/pipeline_configs/stages_config_widget_spec.js b/...webapp/WEB-INF/rails.new/spec/webpack/views/pipeline_configs/stages_config_widget_spec.js
@@ -18,6 +18,7 @@
 describe("StagesConfigWidget", () => {
   const m      = require('mithril');
   const Stream = require('mithril/stream');
+  const $             = require('jquery');
 
   require('jasmine-jquery');
 
@@ -78,8 +79,8 @@ describe("StagesConfigWidget", () => {
     expect(pipeline().stages().countStage()).toBe(1);
     const removeStageButton = $root.find('.remove-stage');
     expect(removeStageButton).toHaveClass("remove-disabled");
-    const tooltipText = removeStageButton.attr("tooltiptext");
-    expect(tooltipText).toBe("Cannot delete the only stage in a pipeline");
+    const tooltipId = $(removeStageButton).attr('data-tooltip-id');
+    expect($(`#${tooltipId}`)).toHaveText("Cannot delete the only stage in a pipeline");
   });
 
   function samplePipelineJSON() {

diff --git a/server/webapp/WEB-INF/rails.new/webpack/helpers/form_helper.js.msx b/server/webapp/WEB-INF/rails.new/webpack/helpers/form_helper.js.msx
@@ -21,6 +21,7 @@ const uuid4 = require('uuid/v4');
 const s     = require('string-plus');
 const Mixin = require('models/mixins/model_mixins');
 require('foundation-sites');
+const Stream            = require('mithril/stream');
 
 function deleteKeyAndReturnValue(object, key, defaultValue) {
   if (_.isNil(object)) {
@@ -408,27 +409,29 @@ const f = {
   },
 
   buttonWithTooltip: {
-    oncreate(vnode) {
-      new window.Foundation.Tooltip($(vnode.dom), {tipText: vnode.attrs.tooltipText});
-    },
-
-    onupdate(vnode) {
-      //an elements data.zfPlugin.template gives the tooltip element
-      $(vnode.dom).data().zfPlugin.template.text(vnode.attrs.tooltipText);
-    },
-
-    onremove(vnode) {
-      $(vnode.dom).foundation('destroy');
+    oninit() {
+      this.tooltipId           = uuid4();
+      this.hasFocus            = Stream(false);
+      this.tooltipDisplayStyle = function () {
+        return this.hasFocus() === true ? 'display:block' : 'display:none';
+      };
     },
 
     view(vnode) {
-      const args = vnode.attrs;
-      return (<button type="button"
-                      class={compactClasses(args, vnode.attrs.buttonType)}
-                      {...args}
-                      data-hover-delay={100}>
-        {vnode.children}
-      </button>);
+      const ctrl        = vnode.state;
+      const args        = vnode.attrs;
+      const tooltipText = deleteKeyAndReturnValue(args, "tooltipText");
+      return (
+        <div style="position: relative">
+          <button type="button" onmouseover={ctrl.hasFocus.bind(ctrl, true)}
+                  onmouseout={ctrl.hasFocus.bind(ctrl, false)}
+                  class={compactClasses(args, vnode.attrs.buttonType, 'has-tip')}
+                  {...args}
+                  data-hover-delay={100} data-tooltip-id={this.tooltipId}>
+            {vnode.children}
+          </button>
+          <div id={this.tooltipId} class="tooltip" role="tooltip" style={ctrl.tooltipDisplayStyle()}>{tooltipText}</div>
+        </div>);
     }
   },
 
@@ -454,25 +457,27 @@ const f = {
   },
 
   linkWithTooltip: {
-    oncreate(vnode) {
-      new window.Foundation.Tooltip($(vnode.dom), {tipText: vnode.attrs.tooltipText});
-    },
-
-    onupdate(vnode) {
-      //Update tooltip text
-      $(vnode.dom).data().zfPlugin.template.text(vnode.attrs.tooltipText);
-    },
-
-    onremove(vnode) {
-      $(vnode.dom).foundation('destroy');
+    oninit() {
+      this.tooltipId           = uuid4();
+      this.hasFocus            = Stream(false);
+      this.tooltipDisplayStyle = function () {
+        return this.hasFocus() === true ? 'display:block' : 'display:none';
+      };
     },
 
-    view(vnode) {
-      return (<a href="javascript:void(0)"
-                 class={compactClasses(vnode.attrs)} {...vnode.attrs}
-                 data-hover-delay={100}>
-        {vnode.children}
-      </a>);
+    view (vnode) {
+      const ctrl        = vnode.state;
+      const args        = vnode.attrs;
+      const tooltipText = deleteKeyAndReturnValue(args, "tooltipText");
+      return (
+        <div style="position: relative; display: inline">
+          <a href="javascript:void(0)" onmouseover={ctrl.hasFocus.bind(ctrl, true)} onmouseout={ctrl.hasFocus.bind(ctrl, false)}
+             class={compactClasses(vnode.attrs, 'has-tip')} {...vnode.attrs}
+             data-hover-delay={100} data-tooltip-id={this.tooltipId}>
+            {vnode.children}
+          </a>
+          <div id={this.tooltipId} class="tooltip"  role="tooltip" style={ctrl.tooltipDisplayStyle()}>{tooltipText}</div>
+        </div>);
     }
   },