Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow functional accessors on y, y0, mark and split #838

Closed
nickofthyme opened this issue Sep 30, 2020 · 1 comment · Fixed by #943
Closed

Allow functional accessors on y, y0, mark and split #838

nickofthyme opened this issue Sep 30, 2020 · 1 comment · Fixed by #943
Labels
breaking change :data Data/series/scales related issue enhancement New feature or request released Issue released publicly :xy Bar/Line/Area chart related

Comments

@nickofthyme
Copy link
Collaborator

nickofthyme commented Sep 30, 2020

Describe the solution you'd like
Allow AccessorFn type for yAccessors, y0Accessors, markSizeAccessor and splitSeriesAccessors.

This is currently allowed for the xAccessor

Describe alternatives you've considered
Reformatting the data before passing it to elastic charts.

Additional context
We need to refactor the way that we track the accessors from the string of the accessor to the index order of the original accessor values.

One change is to update the split series map to use an array to match the provided splitSeriesAccessors array.

function getSplitAccessors(datum: Datum, accessors: Accessor[] = []): Map<string | number, string | number> {
const splitAccessors = new Map<string | number, string | number>();
if (typeof datum === 'object' && datum !== null) {
accessors.forEach((accessor: Accessor) => {
const value = datum[accessor as keyof typeof datum];
if (typeof value === 'string' || typeof value === 'number') {
splitAccessors.set(accessor, value);
}
});
}
return splitAccessors;
}

Secondly, we need to use the yAccessor index rather than the string itself to allow use of AccessorFn

const seriesKeys = [...splitAccessors.values(), accessor];
const seriesKey = getSeriesKey({
specId,
yAccessor: accessor,
splitAccessors,
});

which includes updating the getSeriesKey method

export function getSeriesKey({
specId,
yAccessor,
splitAccessors,
}: Pick<XYChartSeriesIdentifier, 'specId' | 'yAccessor' | 'splitAccessors'>): string {
const joinedAccessors = [...splitAccessors.entries()]
.sort(([a], [b]) => (a > b ? 1 : -1))
.map(([key, value]) => `${key}-${value}`)
.join('|');
return `spec{${specId}}yAccessor{${yAccessor}}splitAccessors{${joinedAccessors}}`;
}

⚠️ This is likely a breaking change unless we want to provide backwards compatibility by using the Accessor string when available and the index otherwise.

Related to #808

@markov00
Copy link
Member

markov00 commented Dec 9, 2020

🎉 This issue has been resolved in version 24.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change :data Data/series/scales related issue enhancement New feature or request released Issue released publicly :xy Bar/Line/Area chart related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants