fix(explore): hide advanced analytics for non temporal xaxis #28312
Conversation
justinpark
changed the title
There was a problem hiding this comment.
Thank you for the PR @justinpark. I left some first-pass comments.
| import { isAdhocColumn, isPhysicalColumn } from '@superset-ui/core'; | ||
| import type { ColumnMeta, ControlPanelsContainerProps } from '../types'; | ||
|
|
||
| export default function showIfTimeSeries({ |
There was a problem hiding this comment.
Can we rename this to showTimeRelatedControls?
There was a problem hiding this comment.
@michael-s-molina to clarify and specify the functionality, i'm thinking of use displayWhenTimeRelatedControls.
What do you think?
There was a problem hiding this comment.
displayTimeRelatedControls (without when) works too.
| }, | ||
| }; | ||
|
|
||
| describe('showIfTimeSeries', () => { |
There was a problem hiding this comment.
Can you avoid describe?
| controlPanelSections: [ | ||
| { | ||
| label: t('Advanced analytics'), | ||
| description: t('Advanced analytical post processin'), |
There was a problem hiding this comment.
| description: t('Advanced analytical post processin'), | |
| description: t('Advanced analytics post processing'), |
| @@ -144,4 +144,41 @@ describe('ControlPanelsContainer', () => { | |||
| await screen.findAllByTestId('collapsible-control-panel-header'), | |||
| ).toHaveLength(2); | |||
| }); | |||
|
|
|||
| test('hide ControlPanelSections when its visibility is false', async () => { | |||
There was a problem hiding this comment.
You're also testing if the panel is displayed when the visibility is true.
| test('hide ControlPanelSections when its visibility is false', async () => { | |
| test('visibility of panels is correctly applied', async () => { |
justinpark
force-pushed
the
fix--explore-hide-advanced-analytics-for-non-temporal-xaxis
branch
from
d5e2c33
to
24da739
Compare
|
@justinpark Can you also make sure to ignore or remove the values from the form data in case a user starts with a temporal column, fills the advanced properties and later changes to a categorial x-axis? |
justinpark
force-pushed
the
fix--explore-hide-advanced-analytics-for-non-temporal-xaxis
branch
from
800e989
to
f1278a8
Compare
Done |
justinpark
force-pushed
the
fix--explore-hide-advanced-analytics-for-non-temporal-xaxis
branch
from
f1278a8
to
c213433
Compare
michael-s-molina
added
the
v4.0
(cherry picked from commit 07cd1d8)
| ...form_data, | ||
| ...restoredField, | ||
| }, | ||
| hiddenFormData, |
There was a problem hiding this comment.
A bit too late, but I guess there should be omit(hiddenFormData, fieldNames), otherwise it seems to break any field that was hidden once.
SUMMARY
The superset feature allows users to choose a non-temporal column for the x-axis.
However, when a non-temporal column is selected, advanced analytics and forecast options are not compatible.
In order to address this, UI should dynamically hide these features when a non-temporal column is selected.
This commit introduces the visibility option in the ControlPanelSectionConfig, enabling the hiding of specific groups of controlSets.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
before--hide-non-temporal.mov
After:
after--hide-non-temporal.mov
TESTING INSTRUCTIONS
Choose non-temporal column in x-axis option
Verify that "Advanced analytics" and "Predictive Analytics" sections are hidden
ADDITIONAL INFORMATION