bito-code-review[bot] commented on code in PR #37169:
URL: https://github.com/apache/superset/pull/37169#discussion_r2694969858
##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx:
##########
@@ -62,18 +62,39 @@ const matrixifyControls: Record<string,
SharedControlConfig<any>> = {};
// Dynamically add axis-specific controls (rows and columns)
(['columns', 'rows'] as const).forEach(axisParam => {
const axis: 'rows' | 'columns' = axisParam;
+ const otherAxis: 'rows' | 'columns' = axis === 'rows' ? 'columns' : 'rows';
matrixifyControls[`matrixify_mode_${axis}`] = {
type: 'RadioButtonControl',
label: t(`Metrics / Dimensions`),
- default: 'metrics',
+ default: axis === 'columns' ? 'metrics' : 'dimensions',
options: [
['metrics', t('Metrics')],
['dimensions', t('Dimension members')],
],
renderTrigger: true,
tabOverride: 'matrixify',
+ description: 'Metrics can\'t be used for both rows and columns at the same
time',
visibility: ({ controls }) => isMatrixifyVisible(controls, axis),
+ shouldMapStateToProps: (prevState, state) => {
+ const otherAxisControlName = `matrixify_mode_${otherAxis}`;
+ return (
+ prevState?.controls?.[otherAxisControlName]?.value !==
+ state?.controls?.[otherAxisControlName]?.value
+ );
+ },
+ mapStateToProps: ({ controls }) => {
+ const otherAxisValue = controls?.[`matrixify_mode_${otherAxis}`]?.value;
+ const isMetricsDisabled = otherAxisValue === 'metrics';
+
+ return {
+ options: [
+ { value: 'metrics', label: t('Metrics'), disabled: isMetricsDisabled
},
+ { value: 'dimensions', label: t('Dimension members') },
+ ],
+ };
+ },
+ rerender: [`matrixify_mode_${otherAxis}`],
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent control state handling</b></div>
<div id="fix">
The control disables the 'metrics' option when the other axis has it
selected, but does not change the value if it was already 'metrics', leading to
an inconsistent state where the control shows 'metrics' as selected but
disabled. This could cause chart behavior issues if both axes end up in
'metrics' mode.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
- return {
- options: [
- { value: 'metrics', label: t('Metrics'), disabled:
isMetricsDisabled },
- { value: 'dimensions', label: t('Dimension members') },
- ],
- };
+ const currentValue = controls?.[`matrixify_mode_${axis}`]?.value;
+ const newValue = (currentValue === 'metrics' && isMetricsDisabled)
? 'dimensions' : currentValue;
+ return {
+ value: newValue,
+ options: [
+ { value: 'metrics', label: t('Metrics'), disabled:
isMetricsDisabled },
+ { value: 'dimensions', label: t('Dimension members') },
- ],
- };
```
</div>
</details>
</div>
<small><i>Code Review Run #5911e6</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
