Skip to content
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

Jump to bottom

fix(plugin-chart-echarts): make to allow the custome of x & y axis title margin i… #18947

Merged
merged 8 commits into from
Mar 10, 2022
Merged

fix(plugin-chart-echarts): make to allow the custome of x & y axis title margin i… #18947

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
af227b2
fix(chartviz): make to allow the custome of x & y axis title margin i…
prosdev0107 Feb 25, 2022
51b66c2
fix(chartviz): add eslint radix error in chart.js
prosdev0107 Feb 25, 2022
489b27d
fix(chartviz): change the transformProps in chart plugin & creat helper.
prosdev0107 Feb 28, 2022
f45b605
fix(chartviz): lint fix & chart.js back
prosdev0107 Feb 28, 2022
2334dc0
fix(chartviz): update from master
prosdev0107 Mar 4, 2022
fd505ee
fix(plugin-chart-echarts): make to allow the custom margin of X & y a…
prosdev0107 Mar 4, 2022
3ceb6fb
fix(plugin-chart-echarts): make to change changeNumber to changeInteger
prosdev0107 Mar 4, 2022
3d5a303
fix(plugin-chart-echarts): make to add license & change file name
prosdev0107 Mar 10, 2022
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
10 changes: 6 additions & 4 deletions superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
formatForecastTooltipSeries,
rebaseForecastDatum,
} from '../utils/forecast';
import { convertNumber } from '../utils/helper';
import { defaultGrid, defaultTooltip, defaultYAxis } from '../defaults';
import {
getPadding,
Expand Down Expand Up @@ -125,6 +126,7 @@ export default function transformProps(
yAxisTitleMargin,
yAxisTitlePosition,
}: EchartsTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };

const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
const rebasedData = rebaseForecastDatum(data, verboseMap);
const xAxisCol = verboseMap[xAxisOrig] || xAxisOrig || DTTM_ALIAS;
Expand Down Expand Up @@ -282,8 +284,8 @@ export default function transformProps(
legendMargin,
addXAxisLabelOffset,
yAxisTitlePosition,
yAxisTitleMargin,
xAxisTitleMargin,
convertNumber(yAxisTitleMargin),
convertNumber(xAxisTitleMargin),
);

const legendData = rawSeries
Expand All @@ -304,7 +306,7 @@ export default function transformProps(
xAxis: {
type: xAxisType,
name: xAxisTitle,
nameGap: xAxisTitleMargin,
nameGap: convertNumber(xAxisTitleMargin),
nameLocation: 'middle',
axisLabel: {
hideOverlap: true,
Expand All @@ -322,7 +324,7 @@ export default function transformProps(
axisLabel: { formatter },
scale: truncateYAxis,
name: yAxisTitle,
nameGap: yAxisTitleMargin,
nameGap: convertNumber(yAxisTitleMargin),
prosdev0107 marked this conversation as resolved.
Show resolved Hide resolved
nameLocation: yAxisTitlePosition === 'Left' ? 'middle' : 'end',
},
tooltip: {
Expand Down
5 changes: 5 additions & 0 deletions superset-frontend/plugins/plugin-chart-echarts/src/utils/helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const convertNumber = (value: string | number) => {
prosdev0107 marked this conversation as resolved.
Show resolved Hide resolved
/* eslint radix: ["error", "as-needed"] */
Copy link
Member

Choose a reason for hiding this comment

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

1 more nit - maybe we could just pass radix 10 instead of ignoring the error?

Copy link
Member

Choose a reason for hiding this comment

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

I was wondering the same. I think there's some chance of it parsing some implicit octal radix, but I haven't stopped to try to break it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved with radix 10

if (typeof value !== 'number') return parseInt(value) || 0;
return value;
};