d3plus / d3plus-plot

A reusable javascript x/y plot built on D3.
MIT License
7 stars 0 forks source link

adds labelPosition method to Plot #181

Closed davelandry closed 1 year ago

davelandry commented 2 years ago

This PR adds a new labelPosition config option to all Plot-type charts, currently only being used for Bar shape label placement.

@ffigueroal please review this at your earliest convenience, but I would also like @AlanORB to take a look at this PR as it is a great simple example of adding a new config option:

  1. First step is to chose which Class should have the new labelPosition option. I've chosen the Plot class here, which means that any classes that extend it will also have that option (like BarChart, StackedArea, etc)
  2. This logic is stored at the bottom of all Classes, in an alphabetical list. We're allowing the user to provide a Function or a String, but we do not want to always check if their value is a function when we need the value, so we coerce the user's value into a function using the constant wrapper here, which is called an "identify function" as it change something like "outside" to () => "outside" so that we can always just invoke the user value like a function.
  3. Add the default value into the constructor of the Class (constant("auto").
  4. Hook up the logic that will interpret the user's value (in this case, inside of our already existing outside function).

Closes #180

davelandry commented 2 years ago

@ffigueroal in my mind, that's the expected behavior here. I'm afraid it's going to get complicated trying to measure the text labels like we do with lineLabels, and I would prefer for this to be the behavior (users can change the yDomain to add some extra buffer if they know the approximate maximum size in the data, but I know that may not be ideal).

@AlanORB you needed this in one of our projects, what do you think we should do? If we set "outside", and the tallest Bar doesn't have enough space on top for a label, should we detect this and make space for it or could you set a bigger max for the yDomain?

AlanORB commented 2 years ago

I would expect us to make space, the user explicity asked for all labels above the bars

davelandry commented 2 years ago

😞 okay, I'll see what I can do

AlanORB commented 2 years ago

but, we were talking with @ffigueroal and this would be a great case to explicitly say to the clients that its a d3plus limitation

danielamguerra commented 1 year ago

I believe this option can solve the problem when the bar is horizontal and the text is outside even fitting the bar, as in the image. With this option, would it be possible to force all labels to stay inside?

image

davelandry commented 1 year ago

@danielamguerra it's possible currently to override the default labelBounds logic, which says "put the label outside of the Bar if the Bar is less than 50% of the overall available height". We use an internal function which can be defined externally as:

/**
 * Determines if a Bar label should be placed outside of the Bar.
 * @param {@} d
 * @param {*} i
 * @private
 */
function outside(d, i) {
  if (this._stacked) return false;
  const other = this._discrete.charAt(0) === "x" ? "y" : "x";
  const nonDiscrete = this._discrete.replace(this._discrete.charAt(0), other);
  const range = this[`_${nonDiscrete}Axis`]._d3Scale.range();
  const value = this[`_${nonDiscrete}`](d, i);
  const negative = value < 0;
  const zero = this[`_${nonDiscrete}Axis`]._getPosition(0);
  const space = nonDiscrete === "y"
    ? negative ? range[1] - zero : zero - range[0]
    : negative ? zero - range[0] : range[1] - zero;
  const pos = this[`_${nonDiscrete}Axis`]._getPosition(value);
  const size = Math.abs(pos - zero);

  // this last line is the logic you should tweak
  return size < space / 2;
}

And this is how you would use it in a shapeConfig:

import {colorContrast, colorDefaults} from "d3plus-color";
...
shapeConfig: {
  Bar: {
    labelBounds(d, i, s) {

      const padding = 1;

      const width = this._discrete === "y" ? "width" : "height";
      const height = this._discrete === "y" ? "height" : "width";

      const other = this._discrete.charAt(0) === "x" ? "y" : "x";
      const invert = other === "y";
      const nonDiscrete = this._discrete.replace(this._discrete.charAt(0), other);
      const range = this[`_${nonDiscrete}Axis`]._d3Scale.range();
      const space = Math.abs(range[1] - range[0]);
      const negative = this[`_${nonDiscrete}`](d, i) < 0;

      if (outside.bind(this)(d, i)) {
        return {
          [width]: space - s[width],
          [height]: s[height],
          x: invert ? -s.width / 2 : negative ? -space : s.width + padding,
          y: invert ? negative ? s.height + padding : -space : -s.height / 2 + 1
        };
      }
      return {
        [width]: s[width],
        [height]: s[height],
        x: invert ? -s.width / 2 : negative ? this._stacked ? padding : padding - s.width : -padding,
        y: invert ? negative ? this._stacked ? padding - s.height : padding : -s.height + padding : -s.height / 2 + padding
      };

    },
    labelConfig: {
      fontColor(d, i) {
        return outside.bind(this)(d, i)
          ? colorDefaults.dark
          : colorContrast(this._shapeConfig.fill(d, i));
      },
      textAnchor(d, i) {
        const other = this._discrete.charAt(0) === "x" ? "y" : "x";
        const invert = other === "y";
        const nonDiscrete = this._discrete.replace(this._discrete.charAt(0), other);
        const negative = this[`_${nonDiscrete}`](d, i) < 0;
        return invert
          ? "middle"
          : outside.bind(this)(d, i)
            ? negative ? "end" : "start"
            : negative ? "start" : "end";
      },
      verticalAlign(d, i) {
        const other = this._discrete.charAt(0) === "x" ? "y" : "x";
        const invert = other === "y";
        const nonDiscrete = this._discrete.replace(this._discrete.charAt(0), other);
        const negative = this[`_${nonDiscrete}`](d, i) < 0;
        return invert
          ? outside.bind(this)(d, i)
            ? negative ? "top" : "bottom"
            : negative ? "bottom" : "top"
          : "middle";
      }
    }
  }
},
danielamguerra commented 1 year ago

@davelandry, thanks for the answer. But, if the labelPosition option is added, I think the problem in this specific case is solved, forcing the labelPosition to 'inside'.

// Detect user "outside" or "inside" override. const labelPosition = this._labelPosition(d, i); if (labelPosition === "outside") return true; else if (labelPosition === "inside") return false;

Will this option get into a release soon?

davelandry commented 1 year ago

@danielamguerra I see what you mean, I just merged and released this as part of v1.2.5 of d3plus-plot 🚀

@ffigueroal @AlanORB we can improve the logic at a later time to add a buffer for labels outside of the max bar.

danielamguerra commented 1 year ago

image I also forced the fontColor to white (if (options.orientation !== 'vertical') { barConfig.labelConfig.fontColor = '#fff' }). It works for me!! Thank you!!