Rtl regresssions in v3 - The sequel

[danielgindi]

This is the sequel to #9194

Expected Behavior

rtl: true in legend with onClick event handlers should hit test the correct label position

Current Behavior

It hit tests the opposite label

Possible Solution

Find the missing piece. Some function out there does not use the rtl helper

Steps to Reproduce

https://codepen.io/danielgindi/pen/rNmBKzP

Context

#9194

Environment

• Chart.js version: 3.4.0
• Chrome 91.0.4472.77

[danielgindi]

@etimberg Originally the hitbox was adjusted here: https://github.com/chartjs/Chart.js/blob/2.9/src/plugins/plugin.legend.js#L493

Perhaps it should be adjusted here: https://github.com/chartjs/Chart.js/blob/master/src/plugins/plugin.legend.js#L246

[etimberg]

I believe that's the right spot to edit for this fix. Similarly, line 258 should be verified as well

[etimberg]

I'm looking into this. It looks like the labels are drawn 3, 2, 1 but the hitboxes are drawn 1, 2, 3. I'm not exactly sure why yet, but making progress.

[etimberg]

Ok, made some progress. During draw we implicitly reverse the positions to draw each row from right to left. This isn't really a problem, however in the hit test code we assume that hitboxes[i] corresponds to legendItems[i] but this isn't the case.

I think a solution here would be to update the order of the hitboxes during draw so that this case is true. Longer term, I think we need to redo the data model of the legend so that all of the RTL handling occurs during fit and the draw becomes a very simple loop. At the same time, we could simplify the internal state such that there is only a single array rather than two arrays that have to be kept in sync. This change is likely breaking so would need to wait for V4.

@kurkle thoughts?

[danielgindi]

@etimberg note that in v2 there was a single loop which generated a single array of positions

[danielgindi]

@etimberg almost there!
The ordering of the hitboxes is now correct, but their widths are inverted.
So if you have

[-----4-------] [--3--] [--------------2-------------] [-----1-----]

it will hit like this:

[-----4-----] [--------------3-------------] [--2--] [-----1-------]

[danielgindi]

@etimberg

Correct rtl version:

  adjustHitBoxes() {
    const me = this;
    if (!me.options.display) {
      return;
    }
    const titleHeight = me._computeTitleHeight();
    const {legendHitBoxes: hitboxes, options: {align, labels: {padding}, rtl}} = me;
    const rtlHelper = getRtlAdapter(rtl, me.left, me.width);
    if (this.isHorizontal()) {
      let row = 0;
      let left = _alignStartEnd(align, me.left + padding, me.right - me.lineWidths[row]);
      for (const hitbox of hitboxes) {
        if (row !== hitbox.row) {
          row = hitbox.row;
          left = _alignStartEnd(align, me.left + padding, me.right - me.lineWidths[row]);
        }
        hitbox.top += me.top + titleHeight + padding;
        hitbox.left = rtlHelper.leftForLtr(rtlHelper.x(left), hitbox.width);
        left += hitbox.width + padding;
      }
    } else {
      let col = 0;
      let top = _alignStartEnd(align, me.top + titleHeight + padding, me.bottom - me.columnSizes[col].height);
      for (const hitbox of hitboxes) {
        if (hitbox.col !== col) {
          col = hitbox.col;
          top = _alignStartEnd(align, me.top + titleHeight + padding, me.bottom - me.columnSizes[col].height);
        }
        hitbox.top = top;
        hitbox.left += me.left + padding;
        hitbox.left = rtlHelper.leftForLtr(rtlHelper.x(hitbox.left), hitbox.width);
        top += hitbox.height + padding;
      }
    }
  }

No rtl conditioning, just pass through the rtl helper.