[charts] HTML Labels

[JCQuintas]

Note: The contents of this PR are marked as @ignore only for now.

The goal is to make the actual Legend PR simpler.

These are the smaller building blocks for the Legend and Tooltip

Regarding why not go for ChartsCircleLabelMark+ChartsSquareLabelMark+.... While it makes each component slightly simpler, it increases the surface area of our components to our users, and the code is mostly the same, so it would require a lot of duplications.

My api target goal is that the eventual labelMark property can be set on the series and series.data for pie charts, while only on the series for other types.

This would accepts our defined label types of 'square' | 'circle' | 'line', but also allow "custom" or any other string the user wants, so they can create custom mark handlers and control it through the series. Eg:

Example custom mark

const MyCustomLegend = () => {
  const { items } = useLegend();
  return (
    <div>
      {items.map((item) => {
        if (item.markType === 'star') {
          return <div key={item.label}>{item.label} - 🌟</div>;
        }

        return <ChartsLabelMark key={item.label} type={item.markType} />;
      })}
    </div>
  );
};

<ChartDataProvider
  height={300}
  width={300}
  series={[
    { 
      // 'star'
      labelMarkType: 'star',
      type: 'line', label: 'Line 1', data: [10, 20] 
    },
    { type: 'line', label: 'Line 2', data: [50, 40] },
  ]}
>
  <ChartsSurface />
  <MyCustomLegend />
</ChartDataProvider>

Test Sandbox

https://codesandbox.io/p/sandbox/html-labels-15813-pt8rwp

[mui-bot]

Deploy preview: https://deploy-preview-15813--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 436b169

[codspeed-hq]

CodSpeed Performance Report

Merging #15813 will not alter performance

_{Comparing JCQuintas:html-labels (436b169) with master (05b5028)}

Summary

✅ 6 untouched benchmarks

[JCQuintas]

We currently use theme colors for this, but the designs suggest other colors. I'm not sure how to proceed. If we follow the design, where do these colors come from?

https://www.figma.com/design/upttiqJUQYGUu3xo6w7COx/Design-refinement?node-id=45-334&node-type=frame&t=kteCWeOvyprNETmc-0

[JCQuintas]

Not sure this should be here or in other packages. Left here for now as we have full control of our own package... 🙃

The idea is to handle all the boilerplate we do for theme and default props to before the component even receives them. Simplifying the business logic of the component itself.

Ofc it is not a silver bullet, but it should handle simple cases like the components in this PR. Name/functionality up to discussion.

[alexfauquette]

it increases the surface area of our components to our users, and the code is mostly the same, so it would require a lot of duplications.

About the surface of components provided, it would just be similar to what MUI is doing for material icons

For the code duplication. It's still possible to have a wrapper for the duplicate part. Something like the SvgIcon.

It may be because I don't see how you will make the type property extendable.

If you allow ChartsLabelMark to be a slot, then devs will have to do something like that to be able to get both the default and their custom

const CustomLableMark = (props) => props.type === "line" ? <ChartsLabelMark {...props} /> : <MyCustomSVG {...props} />

If you give access to what is inside <svg/> you might prevent them from using HTML

If you give access to the SVG itself. You're fairly similar to the SvgIconCompoent. And I would be infavor of exporting the wrapper and one component per icon

[alexfauquette]

WHat about removing this gradientId and directly rendering the ChartsContinuousGradientProps inside the component. Like

function ChartsLabelGradient(){
  const id = useId()
  return <svg>
    <ChartsContinuousGradientProps  id={id}/>
    <rect fill={`url(#${id})`} />
  </svg>
}

Otherwise you get a kind of duplicate information. If you want to switch from row to column, you need to define again the gradient

gradientId="paint_0_row" direction="row"

[JCQuintas]

I tried it at first, my main gripe with adding the gradient inside it that this component loses a bit of flexibility.

I'll try something new and report back

[JCQuintas]

I can work by removing the hardcoded sizes in the svg and adding some CSS

[JCQuintas]

updated the sandbox

[alexfauquette]

That line is a bit weird

[JCQuintas]

Why?

The main thing here is that the line SVG is actually a rectangle with w=100% & h=100%. We use the div's border radius to add the radius, which simplifies a lot the svgs.

Or do you mean that line WIDTH goes into HEIGHT? I got it from stroke-width 😂

[alexfauquette]

Yes, having somethinf called lineWidth used to define a height feels unexpected

[JCQuintas]

Do you have suggestions? I felt lineSize odd, lineWidth and borderWidth are css, lineHeight is a different thing, could be confusing?

[alexfauquette]

Why having a div inside a styled div 🤔

[JCQuintas]

For the line shape. I use the outside div for width=16px so it takes the size necessary, but the line is thin, so there is no need for the component to take height=16px, this way we can let the Root take the width it needs, then the line takes the height defined by lineWidth

[JCQuintas]

@alexfauquette created a small PR on my own fork to show changes to use a defined SVG shape instead of rectangle.

JCQuintas#8

I'm still more biased towards the current approach though 😆

IMO the dom structure is cleaner and it should be easier to target CSS, since it is not deep.

But it is not as trivial to change the shape anymore, since it is an SVG, users would still need to target the rect to change border radius or line width for example.

[alexfauquette]

Good for me. Could you remove the exports from the two index.ts such that those components do not get exposed before the PR that introduces the HTML legend?

It's not strictly speaking necessary, but, would be nice to check their DX works well with the legend/tooltip before exposing them

[alexfauquette]

Nice, I'm impatient to see the result in the main PR :)

[JCQuintas]

Nice, I'm impatient to see the result in the main PR :)

You can always checkout the PR 😆