react-vis icon indicating copy to clipboard operation
react-vis copied to clipboard

Controlling the highlight component

Open ZKruse opened this issue 5 years ago • 5 comments

Hi! I've been using the new updated Highlight component and really like the improvement now it's been incorporated fully into the library from the previously external solution.

I want to use it as a controlled component wherein a chart showing the frequency of data on a time scale has a drag-able highlight region on it and the results of its highlighted time region controls further displays. Most importantly to accurately allow users to select the range I'm bookending the chart with datetime pickers to precisely select a range as well as providing a clear button (which nulls out the brushArea so nothing is highlighted).

I'm experimenting with implementing this capability and I'm not sure what the best behavior would be between adding a new property to Highlight (e.g. newHighlightArea) which sets the brushArea state in componentWillReceiveProps() or adding a new method along the lines of setHighlightArea(newHighlightArea) which has to be called via a ref to the Highlight component.

I've got a semi-tested implementation of the both of these along with a simplified example of the behavior implemented in the showcase. To illustrate the behavior I set up the top chart with 12 bars representing months worth of data. Data is generated for each month and is then aggregated into a histogram for whatever the current time range is based off the highlighted area. The Highlight [month] button loops through setting the highlighted range to exactly each months start/end values.

controlledhighlight

If this sounds interesting/worthwhile I can touch up and create a pull request for my implementation/example.

ZKruse avatar Sep 30 '18 16:09 ZKruse

Hey @ZKruse,

You've highlighted a solid feature! Just as you note the highlight component doesn't really have a mechanism to be set externally. Both of your solutions sound reasonable, though componentWillRecieveProps is deprecating and placing a ref based callback seems a little brittle. I think your feature is good and important, but maybe needs a more nuanced approach. Facebook seems to be pushing for stuff like this https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html#recommendation-fully-uncontrolled-component-with-a-key ? Given a choice between the options you've delineated I'd probably say choose the componentWillRecieveProps route, however it will need to be changed eventually, so maybe just develop a more long term solution.

Regardless, I'd love to see a PR to this effect

mcnuttandrew avatar Oct 01 '18 01:10 mcnuttandrew

Any news on this PR?

mcnuttandrew avatar Oct 14 '18 18:10 mcnuttandrew

I would lean towards the approach proposed by Dan Abramov and seen in other libraries: If Highlight receives an area prop, it will be in controlled mode and will not use state but propagate changes via its callback functions. If area is not set, it will be in uncontrolled mode and therefore work as it does currently.

jbach avatar Mar 02 '19 13:03 jbach

If you also need this feature and don't want to bother forking the repo, here's a very basic implementation for horizontal controlled highlight using inheritance (in Typescript):


import { getAttributeScale } from "react-vis/dist/utils/scales-utils";
type ControlledHighlightProps = Highlight["props"] & {
  area: [any, any] | null;
};
class ImplControlledHighlight extends Highlight {
  props: ControlledHighlightProps;
  state: {
    brushArea: { bottom: number; left: number; right: number; top: number };
    brushing: boolean;
    dragging: boolean;
  };

  componentDidMount() {
    this.updateArea(this.props.area);
  }
  componentDidUpdate() {
    if (!this.state.brushing && !this.state.dragging) {
      this.updateArea(this.props.area);
    }
  }

  private updateArea(area?: [any, any]) {
    let newLeft = 0;
    let newRight = 0;
    if (area) {
      const [left, right] = area;
      const xScale = getAttributeScale(this.props, "x");

      const { marginLeft } = this.props;

      newLeft = xScale(left) + marginLeft;
      newRight = xScale(right) + marginLeft;
    }

    const { left, right } = this.state.brushArea;
    if (left !== newLeft || right !== newRight) {
      const { innerHeight, marginBottom, marginTop } = this.props;
      const plotHeight = innerHeight + marginTop + marginBottom;

      (this as any).setState({
        brushArea: {
          left: newLeft,
          right: newRight,
          top: 0,
          bottom: plotHeight
        }
      });
    }
  }
}
// workaround lack of types
const ControlledHighlight = ImplControlledHighlight as any;

usage:

      <ControlledHighlight
        enableY={false}
        area={snapshot}
        onBrushEnd={area => {
          setSnapshot([area.left, area.right]);
        }}
      />

wyozi avatar Mar 06 '20 10:03 wyozi

@wyozi, You're solution seem to be a great idea. Question, how to do resolve the below error when i tried importing getAttributeScale

Could not find a declaration file for module 'react-vis/dist/utils/scales-utils'. '/Users/arifrahman/Code/work/synthesis/shift-fe/node_modules/react-vis/dist/utils/scales-utils.js' implicitly has an 'any' type. If the 'react-vis' package actually exposes this module, consider sending a pull request to amend 'https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react-vis `

ayepRahman avatar Mar 11 '21 02:03 ayepRahman