Skip to content

[IGNORE] TimeSeriesChart: small refactor #536

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.

Sign up for GitHub

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

Draft
AntoineThebaud wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −17
Draft

[IGNORE] TimeSeriesChart: small refactor #536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 2 additions & 6 deletions timeserieschart/src/TimeSeriesChartPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ import {
DEFAULT_VISUAL,
THRESHOLD_PLOT_INTERVAL,
QuerySettingsOptions,
LOG_BASE,
} from './time-series-chart-model';
import {
getTimeSeries,
Expand Down Expand Up @@ -122,13 +121,10 @@ export function TimeSeriesChartPanel(props: TimeSeriesChartProps): ReactElement
return merge({}, DEFAULT_VISUAL, props.spec.visual);
}, [props.spec.visual]);

// Use the logBase from yAxis options, defaulting to 'none' if not set
const useLogarithmicBase: LOG_BASE = yAxis?.logBase;

// convert Perses dashboard format to be ECharts compatible
const echartsYAxis = useMemo(() => {
return convertPanelYAxis(yAxis, useLogarithmicBase);
}, [yAxis, useLogarithmicBase]);
return convertPanelYAxis(yAxis);
}, [yAxis]);

const [selectedLegendItems, setSelectedLegendItems] = useState<SelectedLegendItemState>('ALL');
const [legendSorting, setLegendSorting] = useState<NonNullable<LegendProps['tableProps']>['sorting']>();
Expand Down
8 changes: 5 additions & 3 deletions timeserieschart/src/utils/data-transform.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('convertPanelYAxis', () => {
min: 0.1,
max: 1,
};
const echartsAxis = convertPanelYAxis(persesAxis, undefined);
const echartsAxis = convertPanelYAxis(persesAxis);
// Axis label is handled outside of echarts since it is built with a custom React component.
expect(echartsAxis).toEqual({
show: true,
Expand All @@ -75,8 +75,9 @@ describe('convertPanelYAxis', () => {
},
min: 0.1,
max: 1,
logBase: 2,
};
const actualAxisLog2 = convertPanelYAxis(persesAxis, 2);
const actualAxisLog2 = convertPanelYAxis(persesAxis);
// Axis label is handled outside of echarts since it is built with a custom React component.
expect(actualAxisLog2).toEqual({
show: true,
Expand All @@ -88,7 +89,8 @@ describe('convertPanelYAxis', () => {
show: true,
},
});
const actualAxisLog10 = convertPanelYAxis(persesAxis, 10);
persesAxis.logBase = 10;
const actualAxisLog10 = convertPanelYAxis(persesAxis);
// Axis label is handled outside of echarts since it is built with a custom React component.
expect(actualAxisLog10).toEqual({
show: true,
Expand Down
12 changes: 4 additions & 8 deletions timeserieschart/src/utils/data-transform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import {
TimeSeriesChartVisualOptions,
TimeSeriesChartYAxisOptions,
LineStyleType,
LOG_BASE,
} from '../time-series-chart-model';

export type RunningQueriesState = ReturnType<typeof useTimeSeriesQueries>;
Expand Down Expand Up @@ -232,13 +231,10 @@ function findMax(data: LegacyTimeSeries[] | TimeSeries[]): number {
* Converts Perses panel yAxis from dashboard spec to ECharts supported yAxis options.
* Handles both linear and logarithmic scales with appropriate min/max calculations.
*/
export function convertPanelYAxis(
inputAxis: TimeSeriesChartYAxisOptions = {},
useLogarithmicBase: LOG_BASE | undefined
): YAXisComponentOption {
export function convertPanelYAxis(inputAxis: TimeSeriesChartYAxisOptions = {}): YAXisComponentOption {
// Determine the appropriate min value based on scale type and user input
let minValue: YAXisComponentOption['min'];
if (useLogarithmicBase !== undefined) {
if (inputAxis.logBase !== undefined) {
// For logarithmic scales without explicit min:
// Let ECharts auto-calculate the range based on data to avoid issues with
// function-based calculations which can result in improper ranges (e.g., 1-10)
Expand Down Expand Up @@ -277,11 +273,11 @@ export function convertPanelYAxis(
};

// Apply logarithmic scale settings if requested
if (useLogarithmicBase !== undefined) {
if (inputAxis.logBase !== undefined) {
return {
...yAxis,
type: 'log',
logBase: useLogarithmicBase,
logBase: inputAxis.logBase,
};
}

Expand Down