clientIO / joint

A proven SVG-based JavaScript diagramming library powering exceptional UIs
https://jointjs.com
Mozilla Public License 2.0
4.72k stars 853 forks source link

[Bug]: elementTools displacement when applying scaling #2702

Closed alexandernst closed 4 months ago

alexandernst commented 4 months ago

Hi Roman!

JJ 3.6 introduced the scale option in elementTools and I have been playing with that the last couple of days. I think I found a bug. Applying a scale to the elementTools will correctly account for the position (x and y props), but it won't account for the offset.

For example, let's say I have a regular shape and I add an elementTools.Button (32x32 px) to it, with the following props: { x: "100%", y: "0%" }. I'd get this (before and after applying a 0.5 zoom / scale), which is correct:

image

But if I add offset: { x: -16, y: -16 } to the elementTools.Button, I would expect the center of the button to be aligned with the upper right corner of my shape, which happens when I apply 1.0 zoom / scale, but it doesn't happen when I apply 0.5 zoom / scale.

image

I think the offset is not properly calculated when scaling is applied.

Steps to reproduce

  1. Open https://codesandbox.io/p/sandbox/jj4-elementtools-displacement-bug-cj5zw7
  2. Observe the bug

Comment 68 contains the offset prop.

Version

4.0.3

What browsers are you seeing the problem on?

Safari

What operating system are you seeing the problem on?

Mac

kumilingus commented 4 months ago

Hi @alexandernst , this works as expected.

It only transforms (scales) the icon with transform-origin 0,0. It has no effect on the position of the button.

You need to define the icon so that its center is at the origin 0,0.

e.g. <circle r="5"/>, <rect x="-10" y="-10" width="20" height="20"/>

Here's updated button from your example:

const InfoButton = elementTools.Button.extend({
  options: {
    x: "100%",
    y: "0%",
    offset: { x: -16, y: -16 },
    markup: [
      {
        tagName: "rect",
        selector: "background",
        attributes: {
          x: -16,
          y: -16,
          width: 32,
          height: 32,
          fill: "#333045E5",
          rx: 6,
          ry: 6,
        },
      },
      {
        tagName: "path",
        selector: "left",
        attributes: {
          transform: "translate(-16,-16)",
          d: "m 11 11 l 11 11",
          fill: "none",
          strokeLinecap: "round",
          stroke: "#FFFFFF",
          strokeWidth: 1.5,
          pointerEvents: "none",
        },
      },
      {
        tagName: "path",
        selector: "right",
        attributes: {
          transform: "translate(-16,-16)",
          d: "m 11 22 l 11 -11",
          fill: "none",
          strokeLinecap: "round",
          stroke: "#FFFFFF",
          strokeWidth: 1.5,
          pointerEvents: "none",
        },
      },
    ],
  },
});
alexandernst commented 4 months ago

Oh, I see. That makes sense 🤔. Applying a transform to the offset while scaling would be error prone and indeed just moving the markup seems easier and cleaner. Thank you!