deepujain commented on code in PR #44556:
URL: https://github.com/apache/superset/pull/44556#discussion_r4098741064
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx:
##########
@@ -246,9 +246,10 @@ function createAxisControl(axis: 'x' | 'y'):
ControlSetRow[] {
label: t('Logarithmic axis'),
renderTrigger: true,
default: logAxis,
- description: t('Logarithmic axis'),
+ description: t('Logarithmic axis is not available for stacked bars'),
Review Comment:
Addressed in c313d0e0: restored the original description and removed the new
translation entry. The stack warning carries the explanation. Current-head
checks have completed successfully.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/controlPanel.test.ts:
##########
@@ -330,3 +334,56 @@ test('x_axis_time_format should be hidden for numeric
columns', () => {
false,
);
});
+
+const logControls = config.controlPanelSections
+ .flatMap(section => (section && section.controlSetRows) || [])
+ .flat()
+ .filter(isCustomControlItem)
+ .filter(control => control.name === 'logAxis');
+
+test.each([OrientationType.Vertical, OrientationType.Horizontal])(
+ 'prevents logarithmic selection while %s bars are stacked, and restores it
when cleared',
+ orientation => {
+ expect(logControls).toHaveLength(2);
+ const state = {
+ controls: {
+ orientation: { value: orientation },
+ stack: { value: StackControlsValue.Stack },
+ logAxis: { value: false },
+ },
+ } as unknown as ControlPanelsContainerProps;
+ expect(
+ logControls.map(control => control.config.visibility!(state, {})),
+ ).toEqual([false, false]);
+ state.controls.stack.value = null;
+ expect(
+ logControls.filter(control => control.config.visibility!(state, {})),
+ ).toHaveLength(1);
+ state.controls.stack.value = StackControlsValue.Stack;
+ state.controls.logAxis.value = true;
+ // A saved chart with both settings must retain a route to turn log mode
off.
+ expect(
+ logControls.filter(control => control.config.visibility!(state, {})),
+ ).toHaveLength(1);
+ },
+);
+
+test.each<[boolean, StackControlsValue | null, boolean]>([
+ [true, null, true],
+ [false, null, false],
+ [true, StackControlsValue.Stack, false],
+ [false, StackControlsValue.Stack, false],
+])(
+ 'stack control with log=%s and stack=%s is disabled=%s',
+ (logAxis, stack, disabled) => {
+ const state = {
+ controls: { logAxis: { value: logAxis }, stack: { value: stack } },
+ } as unknown as ControlPanelState;
+ expect(
+ config.controlOverrides!.stack!.mapStateToProps!(state, {
+ type: 'SelectControl',
+ value: stack,
+ }),
+ ).toMatchObject({ disabled });
Review Comment:
Addressed in c313d0e0: the four-row test now also checks that warning is a
string with log enabled and null otherwise. Current-head checks have completed
successfully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
