graph: fix graph rendering (cylc#1780)

* There appears to have been a recent change in how multiple browsers
  measure the "bounding box" of SVG elements.
* This caused graph nodes to be measured much larger than the area
  they actually render to.
* This messed with the graph because we feed these dimensions into GraphViz
  when creating the virtual graph causing the nodes in the virtual graph
  to be given too much space. An error which is subsequently translated
  to the SVG graph.
* The changes in "bounding box" measurement appear to affect
  SVG Symbols and rotations.
* This commit removes the use of SVG symbols (and their related "use"
  elements) and moves the rotation from the circle element to a group.
* The use of SVG symbols made it easier for us to capture click events
  from behind the svgPanZoom layer.
* Removing SVG symbols required changes to the way
  click events are captured in the graph. Note in SVG the click events
  land on graphical elements (i.e. children), not their containing groups
  (i.e. parents).

Author: oliver-sanders

cypress/component/cylc-graph-node.cy.js

@@ -67,7 +67,14 @@ const GraphNodeSVG = defineComponent({
   render () {
     return h(
       'svg',
-      { id: 'app', class: 'job_theme--default', width: '100%', height: '100%' },
+      {
+        id: 'app',
+        class: 'job_theme--default',
+        width: '100%',
+        height: '100%',
+        // the "-40" bit is to account for the task modifiers
+        viewBox: '-40,-40,450,150'
+      },
       [
         h(GraphNode, this.$attrs)
       ]
@@ -88,10 +95,10 @@ describe('graph node component', () => {
         props: { task, jobs }
       }
     )
-    // there should be 4 jobs (8 svg nodes)
+    // there should be 4 jobs
     cy.get('.c-graph-node:last .jobs')
       .children()
-      .should('have.length', 8)
+      .should('have.length', 4)
     // there shouldn't be a job overflow indicator
     cy.get('.c-graph-node:last .job-overflow').should('not.exist')
 
@@ -113,10 +120,10 @@ describe('graph node component', () => {
         props: { task, jobs, maxJobs: 4 }
       }
     )
-    // there should be <maxJobs> jobs (<maxJobs * 2 svg nodes)
+    // there should be <maxJobs> jobs
     cy.get('.c-graph-node:last .jobs')
       .children()
-      .should('have.length', 8)
+      .should('have.length', 4)
     // there should be a job overflow indicator with the number of overflow jobs
     cy.get('.c-graph-node:last .job-overflow')
       .should('exist')
@@ -189,7 +196,7 @@ describe('graph node component', () => {
         { overwrite: true, disableTimersAndAnimations: false }
       )
       // check the progress animation
-      cy.get('.c8-task:last .status > .progress')
+      cy.get('.c8-task:last .status .progress')
         // the animation duration should be equal to the expected job duration
         .should('have.css', 'animation-duration', `${MEAN_ELAPSED_TIME}s`)
         // the offset should be set to the "percent" of the expected job duration

cypress/component/cylc-icons.cy.js

@@ -98,7 +98,7 @@ describe('Task component', () => {
         { overwrite: true, disableTimersAndAnimations: false }
       )
         // check the progress animation
-        .get('.c8-task:last .status > .progress')
+        .get('.c8-task:last .status .progress')
         // the animation duration should be equal to the expected job duration
         .should('have.css', 'animation-duration', `${MEAN_ELAPSED_TIME}s`)
         // the offset should be set to the "percent" of the expected job duration

src/components/cylc/GraphNode.vue

@@ -18,43 +18,26 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 <template>
   <g class="c-graph-node">
     <!-- the task icon -->
-    <symbol :id="nodeID" viewBox="-40 -40 140 140">
-      <!--
-        Use a "symbol" for the task node in order to apply a viewBox to it.
-        This both contains it and makes it clickable.
-
-        NOTE: Due to the viewBox we use here the coordinate system ends up
-        offset by -20px. This doesn't impact most things, however, rotations
-        can be sensitive to this change causing the rotated elements to end up
-        in the wrong places. To counteract this we provide the coordinate offset
-        to the task component.
-      -->
-      <SVGTask
-        :task="task.node"
-        :modifierSize="0.5"
-        :startTime="startTime"
-        :coordinateOffset="-20"
-      />
-    </symbol>
-    <use
-      :href="`#${nodeID}`"
-      x="0" y="0"
-      width="150" height="150"
+    <SVGTask
+      :task="task.node"
+      :modifierSize="0.5"
+      :startTime="startTime"
+      viewBox="-40 -40 140 140"
       v-cylc-object="task"
+      x="0" y="0"
     />
 
+    <!-- the label -->
     <g :transform="labelTransform">
-      <!-- the task name -->
       <text
-        x="180" y="70"
+        x="130" y="25"
         font-size="45"
       >
         {{ task.name }}
       </text>
 
-      <!-- the cycle point -->
       <text
-        x="180" y="115"
+        x="130" y="65"
         font-size="30"
       >
         {{ task.tokens.cycle }}
@@ -64,7 +47,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
     <!-- the job(s) -->
     <g
       transform="
-        translate(180, 125)
+        translate(130, 75)
         scale(0.3, 0.3)
       "
     >
@@ -77,23 +60,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
           scale(${ (index === 0) ? mostRecentJobScale : '1' })
         `"
       >
-        <symbol
-          :id="`${nodeID}-${index}`"
+        <Job
+          :svg="true"
+          :status="job.node.state"
           viewBox="0 0 100 100"
-          :class="`job_theme--${jobTheme}`"
-        >
-          <!--
-            Use a "symbol" for job nodes in order to make them clickable.
-            The job theme must be set on the "symbol" for styling to work.
-          -->
-          <job
-            :svg="true"
-            :status="job.node.state"
-          />
-        </symbol>
-        <use
-          :href="`#${nodeID}-${index}`"
-          width="100" height="100"
           v-cylc-object="job"
         />
       </g>
@@ -174,7 +144,7 @@ export default {
       if (this.jobs.length) {
         return ''
       }
-      return 'translate(0, 14)'
+      return 'translate(0, 20)'
     },
     previousJobOffset () {
       // the most recent job is larger so all subsequent jobs need to be bumped

src/components/cylc/SVGTask.vue

@@ -68,16 +68,17 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
         the "stroke-dashoffset" to find the new values for 0% and 100%.
         Then copy these values to the corresponding CSS animation keyframes.
       -->
-      <circle
-        class="progress"
-        cx="50"
-        cy="50"
-        r="16"
-        stroke-width="50"
-        stroke-dasharray="157"
-        :transform="progressTransform()"
-        :style="getRunningStyle()"
-      />
+      <g transform="rotate(-90, 50, 50)">
+        <circle
+          class="progress"
+          cx="50"
+          cy="50"
+          r="16"
+          stroke-width="50"
+          stroke-dasharray="157"
+          :style="getRunningStyle()"
+        />
+      </g>
       <!-- dot
 
         A small dot at the centre of the outline used to represent the preparing
@@ -274,12 +275,6 @@ export default {
       type: Number,
       default: 0.7
     },
-    coordinateOffset: {
-      // You may need to provide this if encorporating this icon into a viewBox
-      // otherwise the progress indicator may end up in the wrong place.
-      type: Number,
-      default: 0
-    }
   },
   methods: {
     getRunningStyle () {
@@ -334,9 +329,6 @@ export default {
         translate(${translation}, ${translation})
       `
     },
-    progressTransform () {
-      return `rotate(-90, ${this.coordinateOffset}, ${this.coordinateOffset})`
-    }
   }
 }
 </script>

src/components/cylc/cylcObject/Menu.vue

@@ -283,8 +283,17 @@ export default {
      */
     onClickOutside (e) {
       this.closeMenu()
-      if (e.target?.getAttribute('data-c-interactive')) {
-        this.showMenu = true
+
+      // check if the thing being clicked on is a child of the thing that the
+      // menu is open for
+      let target = e.target
+      while (target) {
+        if (target?.getAttribute('data-c-interactive')) {
+          // keep the menu open
+          this.showMenu = true
+          break
+        }
+        target = target.parentElement
       }
     },
 

src/styles/index.scss

@@ -58,11 +58,6 @@ html {
   &:hover {
     cursor: pointer;
   }
-
-  > * {
-    // Prevent click handler from thinking any child elements are the target
-    pointer-events: none;
-  }
 }
 
 .position-relative {

src/views/Graph.vue

@@ -69,7 +69,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
         <g
           class="edges"
           :transform="
-            (transpose) ? 'translate(15, 30)' : 'translate(45, 5)'
+            (transpose) ? 'translate(-25, -8)' : 'translate(0, -25)'
           "
         >
           <g