rusackas commented on code in PR #32958:
URL: https://github.com/apache/superset/pull/32958#discussion_r3717350553


##########
superset-frontend/src/dashboard/components/SaveModal.tsx:
##########
@@ -48,6 +48,7 @@ type SaveModalProps = {
   dashboardTitle: string;
   dashboardInfo: Record<string, any>;
   expandedSlices: Record<string, any>;
+  expandAllSlices: boolean;

Review Comment:
   Good catch, it really was dead. Removed the prop, the only caller never even 
passes it.



##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -208,7 +208,10 @@ const Chart = (props: ChartProps) => {
   );
   const isExpanded = useSelector(
     (state: RootState) =>
-      !!(state.dashboardState as JsonObject).expandedSlices?.[props.id],
+      !!(
+        (state.dashboardState as JsonObject).expandedSlices?.[props.id] ??
+        (state.dashboardState as JsonObject).expandAllSlices

Review Comment:
   Added expandAllSlices to DashboardState and DashboardStateShape and dropped 
the as JsonObject cast in Chart.tsx now that it's typed.



##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx:
##########
@@ -146,17 +146,80 @@ test('should render a ChartContainer', () => {
   expect(getByTestId('chart-container')).toBeInTheDocument();
 });
 
-test('should render a description if it has one and isExpanded=true', () => {
-  const { container } = setup(
-    {},
-    {
-      dashboardState: {
-        ...defaultState.dashboardState,
-        expandedSlices: { [props.id]: true },
-      },
-    },
-  );
-  expect(container.querySelector('.slice_description')).toBeInTheDocument();
+describe('no description set', () => {
+  for (const sliceExpanded of [undefined, false, true]) {
+    for (const allExpanded of [undefined, false, true]) {
+      test(`should not render a description, expandedSlices=${sliceExpanded} 
and expandAllSlices=${allExpanded}`, () => {
+        const { container } = setup(
+          {},
+          {
+            dashboardState: {
+              ...defaultState.dashboardState,
+              expandedSlices: { [props.id]: sliceExpanded },
+              expandAllSlices: allExpanded,
+            },
+            sliceEntities: {
+              ...sliceEntities,
+              slices: {
+                [queryId]: {
+                  ...sliceEntities.slices[queryId],
+                  description_markeddown: undefined,

Review Comment:
   Good catch, fixed the typo. It was harmless in this case since the base 
fixture already had an empty description, but worth fixing.



-- 
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]

Reply via email to