Skip to content

Commit

Permalink
requested changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@allyoucanmap
allyoucanmap committed Oct 26, 2023
1 parent 4f7db83 commit 6b68e7f
Show file tree
Hide file tree
Showing 27 changed files with 825 additions and 255 deletions.
37 changes: 28 additions & 9 deletions web/client/components/charts/WidgetChart.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
generateClassifiedData,
legacyChartToChartWithTraces,
parseNumber,
parsePieNoAggregationFunctionData
parsePieNoAggregationFunctionData,
enableBarChartStack
} from '../../utils/WidgetsUtils';

const Plot = React.lazy(() => import('./PlotlyChart'));
Expand Down Expand Up @@ -351,15 +352,16 @@ function getMargins({ isModeBarVisible }) {
b: margin,
l: margin,
r: margin,
pad
pad,
autoexpand: true
};
}

function getLayoutOptions({
cartesian,
xAxisOpts = [],
yAxisOpts = [],
barChartType = 'stack',
barChartType,
height,
width
}) {
Expand Down Expand Up @@ -454,7 +456,7 @@ function getLayoutOptions({
}, {});

return {
barmode: barChartType,
...(barChartType && { barmode: barChartType}),
...yAxises,
...xAxises
};
Expand Down Expand Up @@ -492,7 +494,8 @@ export const toPlotly = (_props) => {
height,
width,
legend,
classifyGeoJSONSync
classifyGeoJSONSync,
cartesian
} = props;
const isModeBarVisible = width > 350;
const traces = props.traces || [];
Expand All @@ -507,22 +510,25 @@ export const toPlotly = (_props) => {
};
const xAxisOpts = castArray(props.xAxisOpts || [{ id: 0 }]).map((opts, idx) => idx === 0 ? opts : { ...opts, xaxis: `x${idx + 1}` });
const yAxisOpts = castArray(props.yAxisOpts || [{ id: 0 }]).map((opts, idx) => idx === 0 ? opts : { ...opts, yaxis: `y${idx + 1}` });
const isBarChartStackEnabled = enableBarChartStack({ traces, xAxisOpts, yAxisOpts });
const barChartType = isBarChartStackEnabled
? props.barChartType || 'group'
: 'group';
return {
layout: {
showlegend: legend ?? false, // Set false when legend is undefined, else pie-chart attempts to display legend
// https://plotly.com/javascript/setting-graph-size/
// automargin: true ok for big widgets.
// small widgets should be adapted accordingly
...getLayoutOptions({
cartesian: props.cartesian,
cartesian,
xAxisOpts,
yAxisOpts,
barChartType: props.barChartType,
barChartType,
height,
width
}),
margin: getMargins({ isModeBarVisible }),
autoexpand: false,
autosize: false,
height,
width,
Expand Down Expand Up @@ -577,7 +583,20 @@ export const toPlotly = (_props) => {
...domainProperty
});
return [ ...acc, ...(isArray(data) ? data : [data]) ];
}, []),
}, []).map((trace, idx) => {
if (traces.length > 1 && !isBarChartStackEnabled && trace?.type === 'bar') {
// in case isBarChartStackEnabled is false we currently have two condition
// - a single bar trace is available
// - multiple bar traces with multiple axis are available
// in the second case we should add an group offset to correctly position them
// and visualize them overlapping
return {
...trace,
offsetgroup: idx + 1
};
}
return trace;
}),
config: {
displayModeBar: isModeBarVisible, // minimal to display 8 tools.
modeBarButtonsToRemove: [
Expand Down
42 changes: 22 additions & 20 deletions web/client/components/charts/__tests__/WidgetChart-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual(['#08306b', '#3787c0', '#abd0e6', '#f7fbff']);
// LAYOUT
expect(layout.margin).toEqual({t: 25, b: 8, l: 8, r: 8, pad: 4}); // Modified margin based on width
expect(layout.margin).toEqual({t: 25, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // Modified margin based on width
expect(layout.legend).toBeTruthy();
expect(layout.legend).toEqual({x: 1.05, y: 0.5}); // Legend option to right and centered based on width
});
Expand Down Expand Up @@ -352,7 +352,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual(['#00ff00', '#00ff00']);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
describe('1) Pie chart - Color coded custom classifications with absolute values', () => {
it('custom classified colors - using custom labels and colors only 1', () => {
Expand Down Expand Up @@ -381,7 +381,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual(['#ff0000', '#0000ff']);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
it('custom classified colors - using default labels and colors', () => {
const autoColorOptions = {
Expand Down Expand Up @@ -415,7 +415,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual(['#ff0000', '#0000ff', '#00ff00', '#00ff00']);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
it('custom classified colors - using default labels and colors, wrong order', () => {
const classification = UNLABELLED_CLASSIFICATION_5_ORDERED;
Expand Down Expand Up @@ -449,7 +449,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual([ '#0000ff', '#0000ff', '#00FF00', '#00FF00', '#ff0000', '#ff0000']);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
it('custom classified colors - using templatized labels and custom colors only - pie charts', () => {
const autoColorOptions = { defaultCustomColor: "#00ff00", defaultClassLabel: "", classification: PIE_CHART_TEMPLATE_LABELS_CLASSIFICATION, name: 'global.colors.custom' };
Expand Down Expand Up @@ -477,7 +477,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual([ '#ff0000', '#0000ff']);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
it('custom color ramp', () => {
const autoColorOptions = {
Expand Down Expand Up @@ -530,7 +530,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual([ '#ff0000', '#0000ff', '#0000ff', '#0000ff']);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
it('custom classified colors - using default labels and colors', () => {
const autoColorOptions = { defaultCustomColor: "#00ff00", defaultClassLabel: "Default", rangeClassification: UNLABELLED_RANGE_CLASSIFICATION, name: 'global.colors.custom' };
Expand Down Expand Up @@ -558,7 +558,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual([ '#ff0000', '#0000ff', '#0000ff', '#0000ff']);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
it('custom classified colors - using templatized labels and custom colors only - pie charts', () => {
const autoColorOptions = { defaultCustomColor: "#00ff00", defaultClassLabel: "", rangeClassification: PIE_CHART_TEMPLATE_LABELS_RANGE_CLASSIFICATION, name: 'global.colors.custom' };
Expand Down Expand Up @@ -586,7 +586,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
expect(data[0].marker.colors).toEqual([ '#ff0000', '#00ff00', '#00ff00', '#00ff00' ]);
// LAYOUT
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4}); // fixed margins
expect(layout.margin).toEqual({t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true}); // fixed margins
});
});
describe('Line/Bar chart common features', () => {
Expand All @@ -610,7 +610,7 @@ describe('Widget Chart: data conversions ', () => {
// LAYOUT

// minimal margins, bottom automatic
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });

// colors generated are the defaults, generated on series (1 color for series, so 1)
// expect(layout.colorway).toEqual(defaultColorGenerator(1, COLOR_DEFAULTS));
Expand Down Expand Up @@ -640,7 +640,7 @@ describe('Widget Chart: data conversions ', () => {
}, ({ layout }) => {
// bottom margin is optimized
expect(layout.yaxis.showgrid).toBe(true);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});

});
Expand Down Expand Up @@ -702,7 +702,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
const classColor = LABELLED_CLASSIFICATION.filter(item => item.value === SPLIT_DATASET_2.data[i][0][CLASSIFICATIONS.dataKey])[0].color;
expect(trace.marker.color).toBe(classColor);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});
});
it('custom classified colors - using default labels and colors', () => {
Expand Down Expand Up @@ -736,7 +736,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
const classColor = classification.filter(item => item.value === SPLIT_DATASET_3.data[i][0][CLASSIFICATIONS.dataKey])[0]?.color ?? autoColorOptions.defaultCustomColor;
expect(trace.marker.color).toBe(classColor);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});
});
it('custom classified colors - using default labels and colors, wrong order', () => {
Expand Down Expand Up @@ -771,7 +771,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
const classColor = classification.filter(item => item.value === SPLIT_DATASET_5_ORDERED.data[i][0][CLASSIFICATIONS.dataKey])[0]?.color ?? autoColorOptions.defaultCustomColor;
expect(trace.marker.color).toBe(classColor);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});
});

Expand Down Expand Up @@ -802,7 +802,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
const classColor = TEMPLATE_LABELS_CLASSIFICATION.filter(item => item.value === SPLIT_DATASET_2.data[i][0][CLASSIFICATIONS.dataKey])[0].color;
expect(trace.marker.color).toBe(classColor);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});
});
it('custom bar color', () => {
Expand Down Expand Up @@ -860,7 +860,7 @@ describe('Widget Chart: data conversions ', () => {
({min, max}) => trace.y[0] >= min && trace.y[0] < max
)[0].color;
expect(trace.marker.color).toBe(classColor);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});
});
it('custom classified colors - using default labels and colors', () => {
Expand Down Expand Up @@ -894,7 +894,7 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
const classColor = UNLABELLED_RANGE_CLASSIFICATION[i].color;
expect(trace.marker.color).toBe(classColor);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});
});

Expand Down Expand Up @@ -929,12 +929,12 @@ describe('Widget Chart: data conversions ', () => {
// colors are those defined by the user
const classColor = TEMPLATE_LABELS_RANGE_CLASSIFICATION[i].color;
expect(trace.marker.color).toBe(classColor);
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4 });
expect(layout.margin).toEqual({ t: 8, b: 8, l: 8, r: 8, pad: 4, autoexpand: true });
});
});
});
describe('color coded/custom classified Bar chart - common features', () => {
it('default classified bar chart type is stacked', () => {
it('default classified bar chart type is group', () => {
const autoColorOptions = { defaultCustomColor: "#00ff00", defaultClassLabel: "Default", classification: LABELLED_CLASSIFICATION, name: 'global.colors.custom' };
const { data, layout } = toPlotly({
type: 'bar',
Expand All @@ -947,7 +947,9 @@ describe('Widget Chart: data conversions ', () => {
...DATASET_2
});
expect(data.length).toBe(2);
expect(layout.barmode).toBe('stack');
// we change in group because in some cases is not possible to use stack
// in particular when using multiple traces
expect(layout.barmode).toBe('group');
});
it('change classified bar chart type to grouped', () => {
const autoColorOptions = { defaultCustomColor: "#00ff00", defaultClassLabel: "Default", classification: LABELLED_CLASSIFICATION, name: 'global.colors.custom' };
Expand Down
28 changes: 16 additions & 12 deletions web/client/components/widgets/builder/wizard/ChartWizard.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import emptyChartState from '../../enhancers/emptyChartState';
import errorChartState from '../../enhancers/errorChartState';
import ChartStyleEditor from './chart/ChartStyleEditor';
import ChartTraceEditSelector from './chart/ChartTraceEditSelector';
import TraceAxesOptions from './chart/TraceAxesOptions';
import TraceLegendOptions from './chart/TraceLegendOptions';
import { isChartOptionsValid } from '../../../../utils/WidgetsUtils';

const loadingState = loadingEnhancer(({ loading, data }) => loading || !data, { width: 500, height: 200 });
const hasNoAttributes = ({ featureTypeProperties = [] }) => featureTypeProperties.filter(({ type = "" } = {}) => type.indexOf("gml:") !== 0).length === 0;
Expand All @@ -53,16 +56,6 @@ const enhancePreview = compose(
const PreviewChart = enhancePreview(withResizeDetector(SimpleChart));
const SampleChart = sampleData(withResizeDetector(SimpleChart));

export const isChartOptionsValid = (options = {}, { hasAggregateProcess }) => {
return (
options.aggregationAttribute
&& options.groupByAttributes
// if aggregate process is not present, the aggregateFunction is not necessary. if present, is mandatory
&& (!hasAggregateProcess || hasAggregateProcess && options.aggregateFunction)
|| options.classificationAttribute
);
};

const Wizard = wizardHandlers(WizardContainer);

const renderPreview = ({
Expand Down Expand Up @@ -100,7 +93,7 @@ const renderPreview = ({

const StepHeader = ({step} = {}) => (
<div className="ms-wizard-form-separator">
<Message msgId={`widgets.${step === 0 ? 'chartOptionsTitle' : 'widgetOptionsTitle'}`}/>
<Message msgId={`widgets.${step === 0 ? 'advanced.traceData' : 'widgetOptionsTitle'}`}/>
</div>
);

Expand Down Expand Up @@ -158,11 +151,13 @@ const ChartWizard = ({
<ChartTraceEditSelector
data={data}
editing={step === 0}
error={!!errors?.[selectedTrace?.layer?.name]}
onChange={onChange}
onAddChart={() => toggleLayerSelector(true)}
disableMultiChart={!dashBoardEditing}
tab={tab}
setTab={setTab}
hasAggregateProcess={hasAggregateProcess}
>
<div className="ms-wizard-chart-preview" style={{
position: 'relative',
Expand Down Expand Up @@ -201,6 +196,7 @@ const ChartWizard = ({
layer={selectedTrace?.layer}
disableLayerSelection={!dashBoardEditing}
showTitle={false}
error={!!errors?.[selectedTrace?.layer?.name]}
onChangeLayer={dashBoardEditing ? () => toggleLayerSelector({
key: 'chart-layer-replace',
chartId: selectedChart?.chartId,
Expand All @@ -217,7 +213,7 @@ const ChartWizard = ({
onChange(`charts[${selectedChart?.chartId}].traces[${selectedTrace.id}].${key}`, value)
}
/>
<ChartLayoutOptions
<TraceAxesOptions
data={data}
onChange={onChange}
/>
Expand All @@ -228,6 +224,14 @@ const ChartWizard = ({
onChange(`charts[${selectedChart?.chartId}].traces[${selectedTrace.id}].${key}`, value)
}
/>
<TraceLegendOptions
data={data}
onChange={onChange}
/>
<ChartLayoutOptions
data={data}
onChange={onChange}
/>
</>
),
axis: (
Expand Down
Loading

0 comments on commit 6b68e7f

Please sign in to comment.