Breaking changes on 4.0.0-rc1: hard to overrides fabricJs

[DefeninMaxime]

Hello,

I updated to 4.0.0-rc1. A lot of things are broken in my developments that extend your library. So I'm looking to fix this.

1 ) But I can't properly overrides method of controls.actions.js without copy/past the entirely file.. And it's mandatory to redefine all default_controls.js to take care about overrides... I search to overrides behavior of scaleX/Y/Equally of Textbox.

Somethings is scheduled to help developers to overrides easier the lib ? Or I miss something ?

2 ) Can you explain why the lib export some methods to fabric.controlsUtils but at the execution this methods are in fabric.controlHandlers ? (term not found in the code of lib)

Thanks to your help.

[asturur]

The idea is to gather feedback on how those controls handlers are used.
Some developers already asked for some functions to be exposed and i did so.

Some functions in controls.actions.js are not meant to be reused for different scopes ( like the scaling logic, either you use it for scaling or nothing else ).

Ideally we can build fabricJS without controls entirely, and you add your own file that attach your own controls, we do not want people to override methods and then be locked to old version, but instead, let them add their own functionality on their project OR in external reusable libraries.

2. in the latest changes i renamed fabric.controlHandlers into fabric.controlsUtils. You should not find anymore reference to fabric.controlHandlers in the codebase, or there is a bug.

[asturur]

Can you be more specific on what kind of control you want to change and that you can't do with the actual latest version of the library? ( 4.0 rc1)
Make a concrete example so that i can understand where the problem is.

[DefeninMaxime]

The actual behavior of Textbox for the scaleY or Equally is increase fontsize. Or I want to increase the size of the box without change fontsize.

To do this, only change on followed method is necessary. So I must copy several function and redefine controls to be able to do this.

function scaleObject(eventData, transform, x, y, options) {

// Overrides A
    let modifyWidthOrHeightInsteadOfScale = false;
    if (transform.target instanceof fabric.Textbox) {
      modifyWidthOrHeightInsteadOfScale = true;
    }
// End

    options = options || {};
    var target = transform.target,
      lockScalingX = target.lockScalingX, lockScalingY = target.lockScalingY,
      by = options.by, newPoint, scaleX, scaleY, dim,
      scaleProportionally = scaleIsProportional(eventData, target),
      forbidScaling = scalingIsForbidden(target, by, scaleProportionally),
      signX, signY;

    if (forbidScaling) {
      return false;
    }
    newPoint = getLocalPoint(transform, transform.originX, transform.originY, x, y);
    signX = sign(newPoint.x);
    signY = sign(newPoint.y);
    if (!transform.signX) {
      transform.signX = signX;
    }
    if (!transform.signY) {
      transform.signY = signY;
    }

    if (target.lockScalingFlip &&
      (transform.signX !== signX || transform.signY !== signY)
    ) {
      return false;
    }

    dim = target._getTransformedDimensions();
    const original = transform.original;
    // missing detection of flip and logic to switch the origin
    if (scaleProportionally && !by) {
      // uniform scaling
      var distance = Math.abs(newPoint.x) + Math.abs(newPoint.y),

        originalDistance = Math.abs(dim.x * original.scaleX / target.scaleX) +
          Math.abs(dim.y * original.scaleY / target.scaleY),
        scale = distance / originalDistance, hasScaled;
      scaleX = original.scaleX * scale;
      scaleY = original.scaleY * scale;
    } else {
      scaleX = Math.abs(newPoint.x * target.scaleX / dim.x);
      scaleY = Math.abs(newPoint.y * target.scaleY / dim.y);
    }
    // if we are scaling by center, we need to double the scale
    if (transform.originX === CENTER && transform.originY === CENTER) {
      scaleX *= 2;
      scaleY *= 2;
    }
    if (transform.signX !== signX) {
      transform.originX = opposite[transform.originX];
      scaleX *= -1;
      transform.signX = signX;
    }
    if (transform.signY !== signY) {
      transform.originY = opposite[transform.originY];
      scaleY *= -1;
      transform.signY = signY;
    }
    // minScale is taken are in the setter.
    const oldScaleX = target.scaleX, oldScaleY = target.scaleY;

    const calc = (prop, scale) => {
      return target.get(prop) * scale;
    };

// Overrides B
    if (!by) {
      if (modifyWidthOrHeightInsteadOfScale) {
        !lockScalingX && target.set('width', calc('width', scaleX)) && target.set('scaleX', original.scaleX);
        !lockScalingY && target.set('height', calc('height', scaleY)) && target.set('scaleY', original.scaleY);
      } else {
        !lockScalingX && target.set('scaleX', scaleX);
        !lockScalingY && target.set('scaleY', scaleY);
      }
    } else {
      if (modifyWidthOrHeightInsteadOfScale) {

        if ('x' === by) {
          target.set('width', calc('width', scaleX)) && target.set('scaleX', original.scaleX)
        }

        if ('y' === by) {
          target.set('height', calc('height', scaleY)) && target.set('scaleY', original.scaleY)
        }
      } else {
        // forbidden cases already handled on top here.
        by === 'x' && target.set('scaleX', scaleX);
        by === 'y' && target.set('scaleY', scaleY);
      }
    }
// end
    hasScaled = oldScaleX !== target.scaleX || oldScaleY !== target.scaleY;
    if (hasScaled) {
      fabric.controlHandlers.fireEvent('scaling', commonEventInfo(eventData, transform, x, y));
    }
    target.canvas.requestRenderAll();
    return hasScaled;
  }

[asturur]

I understand.
This is not how i think you would use the api when i imagined it.
The action handler does one thing, either scale, or either change height, and keep the code simple and reusable.

So eventually you do not touch the scale handler. You leave as it is.

You create an action handler that can change height ( one that change width exist ) and you combine both to change the corner.

You attach this new action handler to the textbox controls or special class controls.

I ll come back with an extended example, now i can't, but i wanted to let you know it can be done and is actually much simpler.

[asturur]

I'm not sure if you read the introduction docs and demos before starting your override:
http://fabricjs.com/controls-api
http://fabricjs.com/custom-control-render

As you have noticed, the Textbox class changes the width of the object when using the scaling control. in fabric 3.x and below , this was done with some code like your, .... if object === 'textbox' then.....

Now it works differently, talking of the the textbox example, the MR and ML controls do change width and they do it with a specific handler, the handler is in fabric.controlsUtils.changeWidth and this handler is built with the changeWidth function wrapped with wrapWithFixedAnchor that you find in fabric.controlsUtils.wrapWithFixedAnchor.

This is change width.

  /**
   * Action handler to change textbox width
   * Needs to be wrapped with `wrapWithFixedAnchor` to be effective
   * @param {Event} eventData javascript event that is doing the transform
   * @param {Object} transform javascript object containing a series of information around the current transform
   * @param {number} x current mouse x position, canvas normalized
   * @param {number} y current mouse y position, canvas normalized
   * @return {Boolean} true if some change happened
   */
  function changeWidth(eventData, transform, x, y) {
    var target = transform.target, localPoint = getLocalPoint(transform, transform.originX, transform.originY, x, y),
        strokePadding = target.strokeWidth / (target.strokeUniform ? target.scaleX : 1),
        newWidth = Math.abs(localPoint.x / target.scaleX) - strokePadding;
    target.set('width', Math.max(newWidth, 0));
    return true;
  }

and this is the wrapper:

  /**
   * Wrap an action handler with saving/restoring object position on the transform.
   * this is the code that permits to obects to keep their position while transforming.
   * @param {Function} actionHandler the function to wrap
   * @return {Function} a function with an action handler signature
   */
  function wrapWithFixedAnchor(actionHandler) {
    return function(eventData, transform, x, y) {
      var target = transform.target, centerPoint = target.getCenterPoint(),
          constraint = target.translateToOriginPoint(centerPoint, transform.originX, transform.originY),
          actionPerformed = actionHandler(eventData, transform, x, y);
      target.setPositionByOrigin(constraint, transform.originX, transform.originY);
      return actionPerformed;
    };
  }

Then this is how we attach controls to objects:
https://github.com/fabricjs/fabric.js/blob/master/src/mixins/default_controls.js
As you see Textbox has its own control set and has 2 different handlers on ML and MR.

Now it seems that you want the corners to change width and height instead of scaling.
so BL, BR, TR, TL.

You need an handler to change height first of all and you can easily create it looking at the one that change width:

  /**
   * Action handler to change textbox HEIGHT
   * Needs to be wrapped with `wrapWithFixedAnchor` to be effective
   * @param {Event} eventData javascript event that is doing the transform
   * @param {Object} transform javascript object containing a series of information around the current transform
   * @param {number} x current mouse x position, canvas normalized
   * @param {number} y current mouse y position, canvas normalized
   * @return {Boolean} true if some change happened
   */
  function changeHeight(eventData, transform, x, y) {
    var target = transform.target, localPoint = getLocalPoint(transform, transform.originX, transform.originY, x, y),
        strokePadding = target.strokeWidth / (target.strokeUniform ? target.scaleY : 1),
        newHeight = Math.abs(localPoint.y / target.scaleY) - strokePadding;
    target.set('height', Math.max(newHeight, 0));
    return true;
  }

Then eventually to reuse code, you want to just combine those together:

function changeWidthAndHeight(eventData, transform, x, y) {
  fabric.controlsUtils.changeHeight(eventData, transform, x, y);
  changeWidth(eventData, transform, x, y);
}

Then you want to wrap it with the wrapper so that the object has a fixed position while you drag the corners

var finalHandler = fabric.controlsUtils.wrapWithFixedAnchor(changeWidthAndHeight);

Now exactly has is done for the object prototype, create 4 new controls A B C D to attach to your textboxes, name does not matter.

var myControlSet = {};
// let's keep the rotation control
myControlSet.rotationControl = fabric.Object.prototype.controls.mtr;
// let's keep the standard change width form textbox control
myControlSet.mr = fabric.Textbox.prototype.controls.mr;
myControlSet.ml = fabric.Textbox.prototype.controls.ml;

// repeat this for br, tl, tr with different x and y values
    myControlSet.bl = new fabric.Control({
      x: -0.5,
      y: 0.5,
      actionHandler: finalHandler,
      cursorStyleHandler: controlsUtils.scaleCursorStyleHandler,
      actionName: 'notImportant',
    });

// attach them to your custom class at app init.

MyDifferentTextbox.prototype.controls = myControlSet;

This is the process to help creat custom interaction without overriding fabricJS code. The interface for actions should be stable and not change with fabricJS versions anymore.

[DefeninMaxime]

Oh greats ! My brain was staying in 3.2.6 when implements this ! :) Very efficient controls API thanks for your help !

I have some notes:

1. In 4.0.0-rc1 published on NPM, wrapWithFixedAnchor is not exported, same for getLocalPoint
2. Why continue to use var instead of let or const?
3. When we override MyCustomObject.prototype.controls, controls are modified for each instance of MyCustomObject, good! But if we override myInstanceOfMyCustomObject.controls, I expect controls are modified only for the instance but this is not the case. It's like override MyCustomObject.prototype.controls. This feature is scheduled or it's expected behavior ?

Thanks lot for the time spent to answer me. It's very pleasant 👍

[asturur]

1: maybe i exported it later than rc1, yesterday i tagget a 4.0.0 official.
You definitely find it now.

2: because fabric was started in 2010, and went on changing one line at time. I am mostly alone in mantaining it and a full rewrite with modern JS + transpilier while is nice is less beneficial than new features, helping with issues, writing docs.

3: so the object controls on the prototype is mutable, so if you do:
myInstanceOfMyCustomObject.controls = newControlSet i expect it to change just for one instance
if you do
myInstanceOfMyCustomObject.controls.mr = newControl i expect it to change for every object, unless you previously swapped all the control set.

This is in the nature of JS and mutation, a cool feature to use carefully, i suggest to swap the whole control set at once, is easier.

The way i use it is that i have, for example, 3 control set. A normal one, a crop one, a perspective one.
And depending what function i activate in an image, i swap the control set on the instance.

in this example you see the control set has 12 controls instead of the classic 9, 4 of which are completely out of the image area.

in this demo here:
http://fabricjs.com/custom-controls-polygon

the controls are as many as the polygon point and let you modify the polygon points ( the code isn't super easy or slim )

[DefeninMaxime]

Your last message give me lots of idea to resolve some problems with my crop system ! 👍

I consider that original message receives explanations, so I close the issue.