codeant-ai-for-open-source[bot] commented on code in PR #38519:
URL: https://github.com/apache/superset/pull/38519#discussion_r2910649346
##########
superset-frontend/src/explore/components/ControlPanelsContainer.tsx:
##########
@@ -816,8 +816,10 @@ export const ControlPanelsContainer = (props:
ControlPanelsContainerProps) => {
// Check if matrixify is enabled in form_data
const matrixifyIsEnabled =
- form_data.matrixify_enable_vertical_layout ||
- form_data.matrixify_enable_horizontal_layout;
+ (form_data.matrixify_mode_rows !== undefined &&
+ form_data.matrixify_mode_rows !== 'disabled') ||
+ (form_data.matrixify_mode_columns !== undefined &&
+ form_data.matrixify_mode_columns !== 'disabled');
Review Comment:
**Suggestion:** The Matrixify tab auto-switch logic ignores the new
`matrixify_enable` switch, so the UI can still jump to the Matrixify tab even
when the global Matrixify toggle is off; gate `matrixifyIsEnabled` on this flag
so auto-switching only happens when Matrixify is actually enabled. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Matrixify tab selected even when Matrixify toggle switched off.
- ⚠️ Users land on mostly empty Matrixify panel, confusing UX.
```
</details>
```suggestion
const matrixifyIsEnabled =
form_data.matrixify_enable === true &&
((form_data.matrixify_mode_rows !== undefined &&
form_data.matrixify_mode_rows !== 'disabled') ||
(form_data.matrixify_mode_columns !== undefined &&
form_data.matrixify_mode_columns !== 'disabled'));
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Note that the global Matrixify toggle control `matrixify_enable` is
defined in
`superset-frontend/src/explore/controlPanels/sections.tsx:322` and used in a
visibility
function at `sections.tsx:339-341` (`if (controls?.matrixify_enable?.value
!== true)
return false;`), so turning this switch off hides Matrixify controls without
changing
row/column mode values.
2. Observe the auto-switch logic in
`superset-frontend/src/explore/components/ControlPanelsContainer.tsx:818-822`
where
`matrixifyIsEnabled` is computed solely from `form_data.matrixify_mode_rows`
and
`form_data.matrixify_mode_columns` being non-`'disabled'`, and the effect at
`ControlPanelsContainer.tsx:825-829` that runs `if (showMatrixifyTab &&
matrixifyIsEnabled) { setActiveTabKey(TABS_KEYS.MATRIXIFY); }`.
3. In
`superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx`
create a
test similar to `"should automatically switch to Matrixify tab when
matrixify becomes
enabled"` (lines 93-137 in the existing test file), but set `form_data` to `{
...props.form_data, viz_type: 'line', matrixify_enable: false,
matrixify_mode_rows:
'metrics' }` before rendering `<ControlPanelsContainer {...props} />` with
the Matrixify
feature flag enabled.
4. Run the test and verify that, despite `matrixify_enable: false` (global
toggle off and
Matrixify controls hidden via `sections.tsx:339-341`), the effect in
`ControlPanelsContainer.tsx:825-829` still sets `activeTabKey` to
`TABS_KEYS.MATRIXIFY`,
so `screen.getByRole('tab', { name: /matrixify/i })` has
`aria-selected="true"`—demonstrating that the auto-switch ignores the global
Matrixify
enable switch.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/ControlPanelsContainer.tsx
**Line:** 818:822
**Comment:**
*Logic Error: The Matrixify tab auto-switch logic ignores the new
`matrixify_enable` switch, so the UI can still jump to the Matrixify tab even
when the global Matrixify toggle is off; gate `matrixifyIsEnabled` on this flag
so auto-switching only happens when Matrixify is actually enabled.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38519&comment_hash=e4f44d7092c8c568a46aeef00365e7179fd22877c535306579f1513b70829788&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38519&comment_hash=e4f44d7092c8c568a46aeef00365e7179fd22877c535306579f1513b70829788&reaction=dislike'>👎</a>
--
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]
