victory icon indicating copy to clipboard operation
victory copied to clipboard

Label `angle` should not affect `dx` and `dy` transform coordinate system

Open broofa opened this issue 2 years ago • 1 comments

Describe the bug When an angle is applied to a label, any accompanying dx and dy offsets get applied in the rotated coordinate system. E.g. if a label is rotated 90°, dx ends up moving the label along the Y-axis rather than the X-axis as expected.

Code Sandbox link https://codesandbox.io/s/vigorous-bash-10u75g

To Reproduce Steps to reproduce the behavior:

  1. Go to code sandbox link above.
  2. Note: rotated labels lie below x-axis as expected
  3. Change line 10 to <VictoryLabel textAnchor="end" angle={-45} dx={100}/>
  4. Note that labels move to the right and upwards, (along the rotated label axis)

Before dx offset: CleanShot 2022-07-16 at 09 57 39@2x

After dx offset: CleanShot 2022-07-16 at 09 58 00@2x

Expected behavior dx and dy label transformations should always be in the chart coordinate plane. I.e. I'd expect the graph to look something liek this:

CleanShot 2022-07-16 at 10 02 07@2x

broofa avatar Jul 16 '22 16:07 broofa

My company is using Victory for a current project, and I've developed a workaround for this issue. I'm not sure if this is a candidate for a Pull Request, or not, but my code looks something like this:

const sine = Math.sin(-1 * degreesToRadians(axisProps.labelAngle));
  const cosine = Math.cos(-1 * degreesToRadians(axisProps.labelAngle));
  const secant = Math.sin(-1 * degreesToRadians(90 - axisProps.labelAngle));
  const cosecant = Math.cos(-1 * degreesToRadians(90 - axisProps.labelAngle));

  const baseLabelOffset = getBaseLabelOffset(
    axisProps.labelPosition,
    axisOrientation,
  );

  if (axisOrientation === 'vertical') {
    return {
      dx: 2 * axisProps.axisSpacing * cosine * -1 - baseLabelOffset * sine,
      dy: 2 * (axisProps.axisSpacing * cosecant) + baseLabelOffset * cosine,
    };
  }

  return {
    dx: baseLabelOffset * cosine - 2 * (axisProps.axisSpacing * sine),
    dy: baseLabelOffset * sine - 2 * (axisProps.axisSpacing * secant),
  };

To explain the variables: we have the baseLabelOffset which affects the positioning of the label relative to the tick mark. There is also the axisProps.axisSpacing, which is intended to represent the space between the axis and the label (either horizontal or vertical). In other words, baseLabelOffset describes the expected behavior of dx (horizontal movement) and axisSpacing describes the expected movement from dy (vertical movement).

I have a hard time explaining how the above code works without a diagram, but in short, it translates the movement along the vector of the label angle into up/down motion. (Which is just another way of saying "it works"-- sorry, I'll try again). A portion of the movement along the label vector has to translated into movement in the perpendicular, and likewise, the inverse proportion has to be translated from the label vector into its perpendicular. Basically.... if you imagine a right triangle extending down from the x-axis with the hypotenuse as the label, this code "flips" that triangle so that the hypotenuse is parallel with the x-axis and now the opposite/adjacent angles translate into "normal" up/down left/right movement.

Please note, this code as-is only works for angles -90 to 90, but that covered our use cases.

In any event, hopefully this is helpful to someone.

If this were to make it into the code base, I think there'd be some questions concerning the API that gets exposed. Is there a prop on the label component that turns this on/off, or is the behavior of dx/dy directly modified, etc?

Thoughts?

gtrabbit avatar Jul 12 '23 15:07 gtrabbit