I intend to use this in a styling demo, will be in another PR to not make this too big

This file became just a "selector" to decide which type of axis component to render.

function ChartsXAxis(inProps: ChartsXAxisProps) {
  const { xAxis, xAxisIds } = useXAxes();

  if (xAxis[inProps.axisId ?? xAxisIds[0]].getGrouping) {
    return <ChartsGroupedXAxis {...inProps} />;
  }

  return <ChartsSingleXAxis {...inProps} />;
}

I've removed a lot of checks/functions that are present in the "Single" axis, like shortenLabels and visibility checks.

I went with a "built-in" props/component as that was easier to start.

But I suppose we could also make it a stand-alone component to be used with slots/composition.

This would allow the props to be passed directly to it.

@alexfauquette thoughts on this?

If tickSize is 6px, does this mean that this group will have a 6px margin top, or will it have 3px margin top + 3px margin bottom? Or is it like flex gap where is only applies to the space in between groups?

From the demo, it seems it's the second one, but my point is that it should probably be explained in these docs as I don't think it will be clear to the user.

I'm a bit confused by your question. tickSize doesn't add any margin. It just ensures the tick has a specified size, then the label is rendered after the tick ends, just like the regular axis 🤔

Changing the tick size changes the labels position in the demo, that's why I thought it was related.

So the label changing position is just a consequence of the tick size being larger?

label' y-position is defined by ticksize + a fix gap

xTicks already seems to be an array:

I wonder if we should return separate TickItem[] and TickLabel[], especially with grouping since tickItems.length !== tickLabels.length.

That way, we wouldn't need ignoreTick nor undefined values.

I think for now it would not be necessary

Like, tickItems is very often not the same tickLabels in band scale, as we always have to add a final tick anyways.

Like, tickItems is very often not the same tickLabels in band scale, as we always have to add a final tick anyways.

Yeah, that's why I was wondering if the should separate them. One array of ticks, one array of tick labels.

We could technically do it since useTicks is an internal hook.
Maybe better to do that in a dedicated PR to avoid mixing too many modification

Yeah, separate PR sounds good

Do we want to label this as preview/unstable for now?

The question is "Are we sure the API will not be modified"

The current one seems robust, so I don't see reasons to mark the feature as preview

Should we mention that this only works for band and point scales? If a user tries to use it with a different scale it'll throw an error

I think it would be nice to have different demos, as they current all use the same dataset, but that can be done as a follow-up.

That would be nice for testing the rendering with Argos.

• One that does not start with Q1 and end with Q4
• One that does start with Q1 and end with Q4
• One that uses point scale instead of band scale

Good catch, there issues with the first/last ticks 🤔

• One that does start with Q1 and end with Q4

• One that does not start with Q1 and end with Q4

We can always use the largest possible tick size at the start&end AG Grid
We can leave as is
We can allow changing the firstTick/lastTick: "largest"|"smallest"|groupIndex

We can always use the largest possible tick size at the start&end AG Grid

Looks like a better first step than the current one which has different behavior between first and last tick 👍

Should we precise that this function overrides the value formatter?

When looking at the config, it feels like the config is the part that will grow, and I don't see what else you will put in grouping

<Chart
  xAxis={[{
    ...,
    grouping: {
      getGrouping,
      config: [{ tickSize: 0 }, { tickSize: 32 }],
    },
  }]}
/>

Maybe another approach could be

<Chart
  xAxis={[{
    ...,
    groups: [
      { getValue: getMonth, tickSize: 0 },
      { getValue: getYear, tickSize: 32 },
    ]
  }]}
/>

The internal processing is similar, but you ensure consistency between the number of groups and their config

🤔 I like the groups approach.

I was going to initially allow other props in grouping, but they did only make sense in the other axis types, but now they are out of scope, it might not be necessary.

We could technically do it since useTicks is an internal hook.
Maybe better to do that in a dedicated PR to avoid mixing too many modification

this will play nicely with the grid-charts integration
with row grouping, we keep path for each row that can easily be mapped to this (mapping and in a reverse order - since we keep the top parent on index 0)

You liked @alexfauquette comment above #18766 (comment) which regards to grouping vs groups, but this confused me on what you would prefer 🤔

Can you clarify?

grouping: {
  getGrouping,
  config: [{ tickSize: 0 }, { tickSize: 32 }],
}
// VS
groups: [
  { getValue: getMonth, tickSize: 0 },
  { getValue: getYear, tickSize: 32 },
]

I see, because it will break the array that I said we could compile pretty easy.

Still, I would prefer

groups: [
  { getValue: getMonth, tickSize: 0 },
  { getValue: getYear, tickSize: 32 },
]

It avoids requirement to track the indexes manually.
The values can still be provided pretty easy by picking the index from our internal array.

Also, in case you want to override defaults for the last index, the first options would have to be something like this?

grouping: {
  getGrouping,
  config: [null, { tickSize: 32 }],
}

@arminmeh yeah that would be the case.

I'll convert it to groups: [] then

Should we check if groups is an array?

Otherwise there might be edge cases like this:

const a = { 'groups': undefined };
'groups' in a; // true

I've added some more reasonable checks

Regarding groups.length > 1

I think it could be more confusing, as in I could just be trying to style the first group in isolation, but then if I add another item in the array everything changes.

I think we can leave it as is for now and check how ppl react

I think it could be more confusing, as in I could just be trying to style the first group in isolation, but then if I add another item in the array everything changes.

True. I'm ok with it as is

Should we also check if length > 0?

People might use an empty array to disable this option. Not sure if there's a big benefit in accepting an empty array here.

I don't see an issue with it

Yeah, separate PR sounds good