Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rotten-parrots-stare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lg-charts/core': minor
---

Adds more customization points to charts including 'axisPointer' prop for ChartTooltip, 'emphasis' prop for Bar, and 'type' prop for XAxis.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# IDE files
.idea
*.iml
.vscode

# Locally-installed dependencies
node_modules/
Expand Down
85 changes: 83 additions & 2 deletions charts/core/src/Chart.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { StoryObj } from '@storybook/react';
import { ChartProps } from './Chart/Chart.types';
import { ChartHeaderProps } from './ChartHeader/ChartHeader.types';
import { ChartTooltipProps } from './ChartTooltip/ChartTooltip.types';
import { Bar, LineProps } from './Series';
import { Bar, BarProps, LineProps } from './Series';
import { makeSeriesData } from './testUtils';
import { ThresholdLineProps } from './ThresholdLine';
import {
Expand Down Expand Up @@ -894,10 +894,10 @@ export const _Line: StoryObj<{}> = {
};

export const _Bar: StoryObj<{}> = {
name: 'Bar',
render: () => {
return (
<Chart>
<ChartTooltip />
{lowDensitySeriesData.map(({ name, data }) => (
<Bar name={name} data={data} key={name} />
))}
Expand All @@ -910,6 +910,7 @@ export const BarStacked: StoryObj<{}> = {
render: () => {
return (
<Chart>
<ChartTooltip />
{lowDensitySeriesData.map(({ name, data }, index) => (
<Bar
name={name}
Expand All @@ -923,6 +924,86 @@ export const BarStacked: StoryObj<{}> = {
},
};

export const BarWithSelfEmphasis: StoryObj<{
emphasis: BarProps['emphasis'];
}> = {
args: {
emphasis: 'self',
},
argTypes: {
emphasis: {
control: 'select',
options: ['self', 'none'],
defaultValue: 'self',
},
},
render: ({ emphasis }) => {
return (
<Chart>
<ChartTooltip />
{lowDensitySeriesData.map(({ name, data }) => (
<Bar name={name} data={data} key={name} emphasis={emphasis} />
))}
</Chart>
);
},
};

export const BarWithNoAxisPointer: StoryObj<{
axisPointer: ChartTooltipProps['axisPointer'];
}> = {
args: {
axisPointer: 'none',
},
argTypes: {
axisPointer: {
control: 'select',
options: ['line', 'none'],
defaultValue: 'none',
},
},
render: ({ axisPointer }) => {
return (
<Chart>
<ChartTooltip axisPointer={axisPointer} />
{lowDensitySeriesData.map(({ name, data }) => (
<Bar name={name} data={data} key={name} />
))}
</Chart>
);
},
};

export const BarWithCategoryAxisLabel: StoryObj<{
xAxisType: XAxisProps['type'];
}> = {
args: {
xAxisType: 'category',
},
argTypes: {
xAxisType: {
control: 'select',
options: ['value', 'category'],
defaultValue: 'category',
},
},
render: ({ xAxisType }) => {
return (
<Chart>
<XAxis type={xAxisType} />
<ChartTooltip />
{lowDensitySeriesData.map(({ name, data }) => (
<Bar
name={name}
data={data.map(([_, value], index) => [index, value])}
key={name}
/>
))}
</Chart>
);
},
};

export const NullValues: StoryObj<{}> = {
render: () => {
return (
Expand Down
11 changes: 10 additions & 1 deletion charts/core/src/ChartTooltip/ChartTooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export function ChartTooltip({
seriesValueFormatter,
seriesNameFormatter,
sort,
axisPointer,
}: ChartTooltipProps) {
const {
chart: {
Expand Down Expand Up @@ -117,14 +118,21 @@ export function ChartTooltip({
/**
* using `extraCssText` instead of `className` because emotion-defined class
* didn't have high-enough specificity
*
* darkreader-ignore-inline-style class helps avoid the darkreader tampering
* with the tooltip styles.
*/
className: CHART_TOOLTIP_CLASSNAME,
className: `${CHART_TOOLTIP_CLASSNAME} darkreader-ignore-inline-style`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this should be built into the component or if we should allow the component to accept a className prop, that way it can be passed in if needed. My thought is that this is MMS specific. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and good point! I changed this as you advised to be more flexible. ChartTooltip component now accepts an optional className prop (commit b3fdac1). MMS can now pass in the class darkreader-ignore-inline-style to avoid dark mode issues.

extraCssText: getRootStylesText(theme),
borderWidth: 0,
padding: 0,
showDelay: 0,
hideDelay: 0,
transitionDuration: 0,
axisPointer: {
type: axisPointer,
},

/**
* Since the formatter trigger is set to 'axis', the seriesData will be
* an array of objects. Additionally, it should contain axis related
Expand All @@ -148,6 +156,7 @@ export function ChartTooltip({
theme,
tooltipPinned,
updateOptions,
axisPointer,
]);

return null;
Expand Down
6 changes: 6 additions & 0 deletions charts/core/src/ChartTooltip/ChartTooltip.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ export interface ChartTooltipProps {
seriesValueFormatter?: (value: OptionDataValue) => ReactNode;
seriesNameFormatter?: (name: SeriesName) => ReactNode;
headerFormatter?: (value: number | string) => ReactNode;
/**
* Controls the type of axis pointer shown with the tooltip. Default is 'line'
* which shows a vertical dashed line on hover. Set to 'none' for bar charts to
* avoid the dashed line from being rendered on top of the bars.
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider defining a default value in the type documentation. The comment states 'Default is 'line'' but the type doesn't enforce this. Either add = 'line' in the component signature or explicitly handle the undefined case in the implementation.

Suggested change
* Controls the type of axis pointer shown with the tooltip. Default is 'line'
* which shows a vertical dashed line on hover. Set to 'none' for bar charts to
* avoid the dashed line from being rendered on top of the bars.
* Controls the type of axis pointer shown with the tooltip.
* If not provided, the default is 'line', which shows a vertical dashed line on hover.
* Set to 'none' for bar charts to avoid the dashed line from being rendered on top of the bars.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The axisPointer parameter now has a default value of 'line' in the component signature (ChartTooltip.tsx line 21), so the behavior matches the documentation πŸ‘

*/
axisPointer?: 'none' | 'line';
}

export interface CallbackSeriesDataPoint extends CallbackDataParams {
Expand Down
24 changes: 12 additions & 12 deletions charts/core/src/Series/Bar/Bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,33 @@ export type BarProps = SeriesProps & {
* Stack name for the series. Series with the same stack name are stacked together.
*/
stack?: string;

/**
* Hover focus behavior for the series.
* - `self`: Upon hovering over a bar, all other bars will be dimmed.
* - `none`: Other bars will not be affected by the hover.
*/
emphasis?: 'self';
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The emphasis prop only accepts 'self' but the documentation mentions 'none' as an option. Consider adding 'none' to the type definition: emphasis?: 'self' | 'none' to match the documented behavior and the story's argTypes.

Suggested change
emphasis?: 'self';
emphasis?: 'self' | 'none';

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! ✨ The emphasis prop now includes both 'self' | 'none' in the type definition (commit b3fdac1).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with copilot here, I think you missed 'none' in the type here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Added 'none' to the type definition as you and Copilot both suggested (commit b3fdac1) πŸŽ‰

};

export const Bar = (props: BarProps) => {
export const Bar = ({ name, data, stack, emphasis }: BarProps) => {
const options = useCallback<
(stylingContext: StylingContext) => EChartSeriesOptions['bar']['options']
>(
stylingContext => ({
clip: false,
stack: props.stack,
stack,
emphasis: {
focus: 'self',
focus: emphasis,
},
itemStyle: {
color: stylingContext.seriesColor,
},
}),
[props.stack],
[stack, emphasis],
);

return (
<Series
type={'bar'}
name={props.name}
data={props.data}
options={options}
/>
);
return <Series type="bar" name={name} data={data} options={options} />;
};

Bar.displayName = 'Bar';
7 changes: 7 additions & 0 deletions charts/core/src/XAxis/XAxis.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ export const XAxisType = {
Log: 'log',
Time: 'time',
Value: 'value',
/**
* The 'category' axis type is suitable for discrete types of data like bar charts,
* where each label should be aligned directly under its corresponding bar. Without
* using 'category', the charting library may automatically determine axis label positions,
* which might not correspond to each bar or data point.
*/
Category: 'category',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was originally included but removed before v1 since it made no sense for line charts. Makes sense though for a bar chart of course! If we're going to add this to the x-axis, should we add it to the y-axis as well? It might not add much value but perhaps for consistency it makes sense.

Copy link
Collaborator Author

@nima-taheri-mongodb nima-taheri-mongodb Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great call on keeping things consistent! I also discovered with more testing that for the 'category' axis type, it's important to always provide the data prop. otherwise, Apache ECharts won't display the category labels correctly.

To improve consistency and avoid code duplication, I factored out all common axis properties and types into a single shared Axis.types.ts file. Now, both XAxisProps and YAxisProps extend a common AxisProps type, which supports the 'category' type.

} as const;
type XAxisType = (typeof XAxisType)[keyof typeof XAxisType];

Expand Down
Loading