[charts] Export MuiBarElement-series as part of barElementClasses

[micmcg]

Summary

Currently barElementClasses only contains root (MuiBarElement-root). It would be helpful if it also exported series (MuiBarElement-series).

Thanks

Examples

No response

Motivation

No response

Search keywords: barElement, series, classes

[michelengelen]

Thanks for taking the time to open an issue for this.
It makes sense to include this, so I will add it to the board.

The only problem I can see is that the series classes are suffixed with the seriesId here:

mui-x/packages/x-charts/src/BarChart/BarElement.tsx

Line 47 in 72cc632

root: ['root', `series-${id}`, isHighlighted && 'highlighted', isFaded && 'faded'],

So just exporting it as a member of the barElementClasses object wouldn't be enough. I guess adding a getter, hook or just a tagged template literal might be the way to go.

WDYT @JCQuintas ?

[micmcg]

Ah sure. From my perspective I was happy to append '-${seriesId}' but I understand if you want a more general solution.

[JCQuintas]

Allowing to pass in a seriesId would probably be a bit complex, since our infra expects the bar classes to be strings.

We could have different behaviours like

// This doesn't play very well with our internals
export interface BarElementClasses {
  series: (seriesId: SeriesId) => string;
}

barElementClasses.series('myId')

// or

// This would work, but it is a bit hidden as a function inside a string.
export interface BarElementClasses {
  series: string & { withId: (seriesId: SeriesId) => string };
}

barElementClasses.series.withId('myId')

@michelengelen @alexfauquette do you know if there are other places in the codebase we use similar patterns? 🤔

[michelengelen]

@michelengelen @alexfauquette do you know if there are other places in the codebase we use similar patterns? 🤔

I know there was a similar pattern in the grid somewhere, but afaik its not used for classes. Those should ideally be static. 🤷🏼

If manually re-creating (in userland) these is an option this can work as well ... in that case we should just export the "base" class for series like the others and then let the user decide if he needs to use this.

On the other hand this can be made into a helper to be used in userland as well, which should work, when we are not using it in the internals, right?

[JCQuintas]

Yeah, just providing the -series class would be simpler

[michelengelen]

In that case i will add the "good first issue" label and give a short diff to get a contributor started:

--- a/packages/x-charts/src/BarChart/BarElement.tsx
+++ b/packages/x-charts/src/BarChart/BarElement.tsx
@@ -18,6 +18,10 @@ export interface BarElementClasses {
   highlighted: string;
   /** Styles applied to the root element if it is faded. */
   faded: string;
+  /** Styles applied to the root element for a specified series.
+   *  To be used like this: `&.${barElementClasses.series}-${seriesId}`.
+   */
+  series: string;
 }

 export type BarElementClassKey = keyof BarElementClasses;
@@ -39,6 +43,7 @@ export const barElementClasses: BarElementClasses = generateUtilityClasses('MuiBarElement', [
   'root',
   'highlighted',
   'faded',
+  'series',
 ]);

 const useUtilityClasses = (ownerState: BarElementOwnerState) => {

cc @JCQuintas

[10tacion]

Hi! Would it be okay if I gave this a try?

If it's alright, I'd also appreciate it if you could add a label to PR #17273.

[michelengelen]

Hi! Would it be okay if I gave this a try?

If it's alright, I'd also appreciate it if you could add a label to PR #17273.

Yep... I added reviewers and the label! Thanks for the contribution! 💪🏻

[bernardobelchior]

Fixed by #17273.
The fix should be out in v8.0.0-beta.4, which should be released later this week.
Thank you @micmcg and @10tacion for your contributions!

[github-actions]

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.