clean up and document

packages/core/src/part.js

@@ -350,6 +350,7 @@ Part.prototype.shorthand = function () {
   return shorthand
 }
 
+/** generate the transform attributes needed for the given part */
 export const generatePartTransform = (x, y, rotate, flipX, flipY, part) => {
   const center = {
     x: part.topLeft.x + (part.bottomRight.x - part.topLeft.x)/2,

packages/core/src/pattern.js

@@ -531,39 +531,6 @@ Pattern.prototype.pack = function () {
       if (this.parts[partId]) {
         let transforms = this.settings.layout.parts[partId]
         this.parts[partId].generateTransform(transforms);
-      //   // Moving
-      //   if (typeof transforms.move === 'object') {
-      //     this.parts[partId].attributes.set(
-      //       'transform',
-      //       'translate(' + transforms.move.x + ', ' + transforms.move.y + ')'
-      //     )
-      //   }
-      //   // Mirrorring
-      //   let center = this.parts[partId].topLeft.shiftFractionTowards(
-      //     this.parts[partId].bottomRight,
-      //     0.5
-      //   )
-      //   let anchor = { x: 0, y: 0 }
-      //   if (transforms.flipX) {
-      //     let dx = anchor.x - center.x
-      //     let transform = `translate(${center.x * -1}, ${center.y * -1})`
-      //     transform += ' scale(-1, 1)'
-      //     transform += ` translate(${center.x * -1 + 2 * dx}, ${center.y})`
-      //     this.parts[partId].attributes.add('transform', transform)
-      //   }
-      //   if (transforms.flipY) {
-      //     let dy = anchor.y - center.y
-      //     let transform = `translate(${center.x * -1}, ${center.y * -1})`
-      //     transform += ' scale(1, -1)'
-      //     transform += ` translate(${center.x}, ${center.y * -1 + 2 * dy})`
-      //     this.parts[partId].attributes.add('transform', transform)
-      //   }
-      //   if (transforms.rotate) {
-      //     let transform = `rotate(${transforms.rotate}, ${center.x - anchor.x}, ${
-      //       center.y - anchor.y
-      //     })`
-      //     this.parts[partId].attributes.add('transform', transform)
-      //   }
       }
     }
   }

sites/shared/components/workbench/layout/draft/index.js

@@ -1,77 +1,3 @@
-/*
- * This React component is a long way from perfect, but it's a start for
- * handling custom layouts.
- *
- * There are two reasons that (at least in my opinion) implementing this is non-trivial:
- *
- * 1) React re-render vs DOM updates
- *
- *    For performance reasons, we can't re-render with React when the user drags a
- *    pattern part (or rotates it). It would kill performance.
- *    So, we don't re-render with React upon dragging/rotating, but instead manipulate
- *    the DOM directly.
- *
- *    So far so good, but of course we don't want a pattern that's only correctly laid
- *    out in the DOM. We want to updat the pattern gist so that the new layout is stored.
- *    For this, we re-render with React on the end of the drag (or rotate).
- *
- *    Handling this balance between DOM updates and React re-renders is a first contributing
- *    factor to why this component is non-trivial
- *
- * 2) SVG vs DOM coordinates
- *
- *    When we drag or rotate with the mouse, all the events are giving us coordinates of
- *    where the mouse is in the DOM.
- *
- *    The layout uses coordinates from the embedded SVG which are completely different.
- *    So we need to make this translation and that adds complexity.
- *
- * 3) Part-level transforms
- *
- *    It's not just that the DOM coordinates and SVG coordinate system is different, each
- *    part also has it's own transforms applied and because of this behaves as if they have
- *    their own coordinate system.
- *
- *    In other words, a point (0,0) in the part is not the top-left corner of the page.
- *    In the best-case scenario, it's the top-left corner of the part. But even this is
- *    often not the case as parts will have transforms applied to them.
- *
- * 4) Flip along X or Y axis
- *
- *    Parts can be flipped along the X or Y axis to facilitate a custom layout.
- *    This is handled in a transform, so the part's coordinate's don't actually change. They
- *    are flipped late into the rendering process (by the browser displaying the SVG).
- *
- *    Handling this adds yet more mental overhead
- *
- * 5) Bounding box
- *
- *    While moving and rotating parts around is one thing. Recalculating the bounding box
- *    (think auto-cropping the pattern) gets kinda complicated because of the reasons
- *    outlined above.
- *
- *    We are currently handling a lot in the frontend code. It might be more elegant to move
- *    some of this to core. For example, core expects the custom layout to set the widht and height
- *    rather than figuring it out on its own as it does for auto-generated layouts.
- *
- *
- *
- *  Known issues
- *
- *  - Rotating gets a little weird sometimes as the part rotates around it's center in the
- *    SVG coordinate system, but the mouse uses it's own coordinates as the center point that's
- *    used to calculate the angle of the rotation
- *
- *  - Moving parts into the negative space (minus X or Y coordinated) does not extend the bounding box.
- *
- *  - Rotation gets weird when a part is mirrored
- *
- *  - The bounding box update when a part is rotated is not entirely accurate
- *
- *
- *  I've sort of left it at this because I'm starting to wonder if we should perhaps re-think
- *  how custom layouts are supported in the core. And I would like to discuss this with the core team.
- */
 import { useEffect, useRef, useState } from 'react'
 import Svg from '../../draft/svg'
 import Defs from '../../draft/defs'
@@ -89,7 +15,6 @@ const Draft = props => {
     if (!layout) {
       // On the initial draft, core does the layout, so we set the layout to the auto-layout
       // After this, core won't handle layout anymore. It's up to the user from this point onwards
-      // FIXME: Allow use the option to clear the layout again
       updateGist(['layout'], {
         ...patternProps.autoLayout,
         width: patternProps.width,
@@ -104,13 +29,6 @@ const Draft = props => {
   const updateLayout = (name, config) => {
     // Start creating new layout
     const newLayout = {...layout}
-    const matrix = svgRef.current.getScreenCTM().inverse();
-
-    // convert topLeft and bottom right from DOM coordinates to svg coordinates
-    if (config.tl) {
-      config.tl = DOMPointReadOnly.fromPoint(config.tl).matrixTransform(matrix)
-      config.br = DOMPointReadOnly.fromPoint(config.br).matrixTransform(matrix);
-    }
     newLayout.parts[name] = config
 
     // Pattern topLeft and bottomRight
@@ -128,15 +46,10 @@ const Draft = props => {
       }
     }
 
-    // move the pages to the top left corner of the viewBox
-    // newLayout.parts.pages.move.x = Math.min(0, topLeft.x);
-    // newLayout.parts.pages.move.y = Math.min(0, topLeft.y)
-
     newLayout.width = bottomRight.x - topLeft.x
     newLayout.height = bottomRight.y - topLeft.y
     newLayout.bottomRight = bottomRight
     newLayout.topLeft = topLeft
-    console.log(newLayout.topLeft, newLayout.bottomRight);
     updateGist(['layout'], newLayout)
   }
 

sites/shared/components/workbench/layout/draft/part.js

@@ -1,3 +1,48 @@
+/*
+ * This React component is a long way from perfect, but it's a start for
+ * handling custom layouts.
+ *
+ * There are a few reasons that (at least in my opinion) implementing this is non-trivial:
+ *
+ * 1) React re-render vs DOM updates
+ *
+ *    For performance reasons, we can't re-render with React when the user drags a
+ *    pattern part (or rotates it). It would kill performance.
+ *    So, we don't re-render with React upon dragging/rotating, but instead manipulate
+ *    the DOM directly.
+ *
+ *    So far so good, but of course we don't want a pattern that's only correctly laid
+ *    out in the DOM. We want to update the pattern gist so that the new layout is stored.
+ *    For this, we re-render with React on the end of the drag (or rotate).
+ *
+ *    Handling this balance between DOM updates and React re-renders is a first contributing
+ *    factor to why this component is non-trivial
+ *
+ * 2) SVG vs DOM coordinates
+ *
+ *    When we drag or rotate with the mouse, all the events are giving us coordinates of
+ *    where the mouse is in the DOM.
+ *
+ *    The layout uses coordinates from the embedded SVG which are completely different.
+ *
+ *    We run `getScreenCTM().inverse()` on the svg element to pass to `matrixTransform` on a `DOMPointReadOnly` for dom to svg space conversions.
+ *
+ * 3) Part-level transforms
+ *
+ *    All parts use their center as the transform-origin to simplify transforms, especially flipping and rotating.
+ *
+ * 4) Bounding box
+ *
+ *    We use `getBoundingClientRect` rather than `getBBox` because it provides more data and factors in the transforms.
+ * We then use our `domToSvg` function to move the points back into the SVG space.
+ *
+ *
+ *  Known issues
+ *  - currently none
+ *
+ *  I've sort of left it at this because I'm starting to wonder if we should perhaps re-think
+ *  how custom layouts are supported in the core. And I would like to discuss this with the core team.
+ */
 import Path from '../../draft/path'
 import Point from '../../draft/point'
 import Snippet from '../../draft/snippet'
@@ -8,6 +53,7 @@ import { drag } from 'd3-drag'
 import { select } from 'd3-selection'
 import { useRef, useState, useEffect} from 'react'
 
+/** buttons for manipulating the part */
 const Buttons = ({ transform, flip, rotate, setRotate, resetPart }) => {
   const letter = 'F'
   const style = { style: {fill: 'white', fontSize: 18, fontWeight: 'bold', textAnchor: 'middle'} }
@@ -34,27 +80,6 @@ const Buttons = ({ transform, flip, rotate, setRotate, resetPart }) => {
   )
 }
 
-const generateTransform = (x, y, rot, flipX, flipY, part) => {
-  // const center = {
-  //   x: part.topLeft.x + (part.bottomRight.x - part.topLeft.x)/2,
-  //   y: part.topLeft.y + (part.bottomRight.y - part.topLeft.y)/2,
-  // }
-  // const dx = part.topLeft.x - center.x
-  // const dy = part.topLeft.y - center.y
-  const transforms = [`translate(${x},${y})`]
-  if (flipX) transforms.push(
-    'scale(-1, 1)',
-  )
-  if (flipY) transforms.push(
-    'scale(1, -1)',
-  )
-  if (rot) transforms.push(
-    `rotate(${rot}, ${center.x}, ${center.y})`
-  )
-
-  return transforms.join(' ')
-}
-
 const Part = props => {
   const { layout, gist, name, part} = props
 
@@ -87,29 +112,33 @@ const Part = props => {
   let translateY = partLayout.move.y
   let partRotation = partLayout.rotate || 0;
   let rotation = partRotation;
-  let flipX = partLayout.flipX ? true : false
+  let flipX = !!partLayout.flipX
-  let flipY = partLayout.flipY ? true : false
+  let flipY = !!partLayout.flipY
-  // let partRect
 
   const center = {
     x: part.topLeft.x + (part.bottomRight.x - part.topLeft.x)/2,
     y: part.topLeft.y + (part.bottomRight.y - part.topLeft.y)/2,
   }
 
+  /** get the delta rotation from the start of the drag event to now */
   const getRotation = (event) => angle(center, event.subject) - angle(center, { x:event.x, y: event.y });
 
   const handleDrag = drag()
+    // subject allows us to save data from the start of the event to use throughout event handing
     .subject(function(event) {
-      return rotate ? { x: event.x, y: event.y } : {x: translateX, y: translateY}
+      return rotate ?
-    })
+      // if we're rotating, the subject is the mouse position
-    .on('start', function(event) {
+      { x: event.x, y: event.y } :
-      // partRect = partRef.current.getBoundingClientRect()
+      // if we're moving, the subject is the part's x,y coordinates
+      {x: translateX, y: translateY}
     })
     .on('drag', function(event) {
       if (rotate) {
         let newRotation = getRotation(event);
+        // reverse the rotation direction one time per flip. if we're flipped both directions, rotation will be positive again
         if (flipX) newRotation *= -1
         if (flipY) newRotation *= -1
+
         rotation = partRotation + newRotation
       }
       else {
@@ -117,14 +146,16 @@ const Part = props => {
         translateY = event.y
       }
 
+      // get the transform attributes
       const transforms = generatePartTransform(translateX, translateY, rotation, flipX, flipY, part);
-      console.log(transforms)
+
       const me = select(this);
       for (var t in transforms) {
         me.attr(t, transforms[t])
       }
     })
     .on('end', function(event) {
+      // save to gist
       updateLayout()
     })
 
@@ -140,8 +171,13 @@ const Part = props => {
   }
   const updateLayout = () => {
     const partRect = partRef.current.getBoundingClientRect();
-    const tl = {x: partRect.left, y: partRect.top};
+    const matrix = partRef.current.ownerSVGElement.getScreenCTM().inverse();
-    const br = {x: partRect.right, y: partRect.bottom};
+
+    const domToSvg = (point) => DOMPointReadOnly.fromPoint(point).matrixTransform(matrix)
+
+    // include the new top left and bottom right to ease calculating the pattern width and height
+    const tl = domToSvg({x: partRect.left, y: partRect.top});
+    const br = domToSvg({x: partRect.right, y: partRect.bottom});
 
     props.updateLayout(name, {
       move: {