Feature/aria label

[boygirl]

Thanks to @ljones87 , tthis PR adds a new ariaLabel prop to all primitive components. This prop can take a string or a function that is expected to return a string. Resulting rendered elements will have aria-label attributes. This PR also adds a a new component, VictoryAccessibleGroup that renders both a g tag and an optional desc tag along with its children. This new component may be used as the groupComponent on any Victory component.

[boygirl]

@ljones87 looks like there are a couple of things failing nps lint.test. (not caught in previous PRs since they weren't against main). Other than that, I think this is good to merge.

[boygirl]

Looks like there are a couple more lint errors. https://travis-ci.com/github/FormidableLabs/victory/jobs/396608334. You can also run this locally with nps check

[ljones87]

haha yeah, addressing those now, just down to fixing all the "magic numbers". Will run all the lints individually before pushing again

[boygirl]

🎉 Once CI passes, I'll get this merged and released as a minor version. Nice work!

[ljones87]

Looks like just one error left, cause by my resolving the lint warnings in victory-brush-line-demo.tsx:

demo/ts/components/victory-brush-line-demo.tsx(118,21): error TS2322: Type 'DataSet[] | (string | null | undefined)[]' is not assignable to type 'string[] | DataSet[]'.
  Type '(string | null | undefined)[]' is not assignable to type 'string[] | DataSet[]'.
    Type '(string | null | undefined)[]' is not assignable to type 'string[]'.
      Type 'string | null | undefined' is not assignable to type 'string'.
        Type 'undefined' is not assignable to type 'string'.

filters and the return on line 106 of victory-brush-line-demo were not being used:

  getActiveDatasets(filters: {}  ********was "unused" ********) {
    // Return the names from all datasets that have values within all filters
    const isActive = (dataset: DataSet, filters: {}): (string | null | undefined)[] => {
      return _.keys(filters).reduce((memo: any, name: any) => {
        if (!memo || !Array.isArray(filters[name])) {
          return memo;
        }
        const point = _.find(dataset.data, (d) => d.x === name);
        return (
          point && Math.max(...filters[name]) >= point.y && Math.min(...filters[name]) <= point.y
        );
      }, true);

      ********below return never reached, thus filters prop was unused ********
      return this.state.datasets
        .map((dataset: DataSet) => {
          return isActive(dataset, filters) && dataset ? dataset.name : null;
        })
        .filter(Boolean);
    };

     ******** always returned this empty array ********
    return [];
  }

Thought a quick fix would work in moving the unused return to the correct scope, thus replacing the empty array return

  getActiveDatasets(filters: {}) {
    // Return the names from all datasets that have values within all filters
    const isActive = (dataset: DataSet, filters: {}): (string | null | undefined)[] => {
      return _.keys(filters).reduce((memo: any, name: any) => {
        if (!memo || !Array.isArray(filters[name])) {
          return memo;
        }
        const point = _.find(dataset.data, (d) => d.x === name);
        return (
          point && Math.max(...filters[name]) >= point.y && Math.min(...filters[name]) <= point.y
        );
      }, true);
    };

    return this.state.datasets
      .map((dataset: DataSet) => {
        return isActive(dataset, filters) && dataset ? dataset.name : null;
      })
      .filter(Boolean);
  }

But it messed up the types! Third times the charm? 🤞

[boygirl]

I was looking at the odd failure that was persisting in CI. nps compile-ts is failing, and I think it was caused by removing this return.

[boygirl]

oops, you already caught it

[boygirl]

🎉 Thanks! I'll get this merged and released this morning.