Dashboard: Improve diff styling (#81509)

* Dashboard: Improve diff styling

* Update public/app/features/dashboard-scene/settings/version-history/DiffGroup.tsx

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

* Fix

* Update

* Update

---------

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

.betterer.results

@@ -2658,9 +2658,6 @@ exports[`better eslint`] = {
       [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"],
       [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "1"]
     ],
-    "public/app/features/dashboard/components/SaveDashboard/SaveDashboardDiff.tsx:5381": [
-      [0, 0, 0, "Styles should be written using objects.", "0"]
-    ],
     "public/app/features/dashboard/components/SaveDashboard/SaveDashboardErrorProxy.tsx:5381": [
       [0, 0, 0, "Styles should be written using objects.", "0"],
       [0, 0, 0, "Styles should be written using objects.", "1"],

public/app/features/dashboard-scene/saving/SaveDashboardDrawer.test.tsx

@@ -77,7 +77,7 @@ describe('SaveDashboardDrawer', () => {
 
       await userEvent.click(await screen.findByLabelText('Tab Changes'));
 
-      expect(await screen.findByText('JSON Model')).toBeInTheDocument();
+      expect(await screen.findByText('Full JSON diff')).toBeInTheDocument();
     });
 
     it('Can save', async () => {

public/app/features/dashboard-scene/settings/version-history/DiffGroup.tsx

@@ -42,16 +42,14 @@ export const DiffGroup = ({ diffs, title }: DiffGroupProps) => {
 };
 
 const getStyles = (theme: GrafanaTheme2) => ({
-  container: css({
+  container: css({}),
-    backgroundColor: theme.colors.background.secondary,
-    fontSize: theme.typography.h6.fontSize,
-    marginBottom: theme.spacing(2),
-    padding: theme.spacing(2),
-  }),
   list: css({
     marginLeft: theme.spacing(4),
   }),
   listItem: css({
     marginBottom: theme.spacing(1),
+    '&:last-child': {
+      marginBottom: 0,
+    },
   }),
 });

public/app/features/dashboard-scene/settings/version-history/DiffTitle.tsx

@@ -19,13 +19,14 @@ export const DiffTitle = ({ diff, title }: DiffTitleProps) => {
 
   return diff ? (
     <>
-      <Icon type="mono" name="circle" className={styles[diff.op]} /> <span className={styles.embolden}>{title}</span>{' '}
+      <Icon type="mono" name="circle" className={styles[diff.op]} size="xs" />{' '}
-      <span>{getDiffText(diff, diff.path.length > 1)}</span> <DiffValues diff={diff} />
+      <span className={styles.embolden}>{title}</span> <span>{getDiffText(diff, diff.path.length > 1)}</span>{' '}
+      <DiffValues diff={diff} />
     </>
   ) : (
     <div className={styles.withoutDiff}>
-      <Icon type="mono" name="circle" className={styles.replace} /> <span className={styles.embolden}>{title}</span>{' '}
+      <Icon type="mono" name="circle" className={styles.replace} size="xs" />{' '}
-      <span>{getDiffText(replaceDiff, false)}</span>
+      <span className={styles.embolden}>{title}</span> <span>{getDiffText(replaceDiff, false)}</span>
     </div>
   );
 };
@@ -56,6 +57,6 @@ const getDiffTitleStyles = (theme: GrafanaTheme2) => ({
     color: theme.colors.success.main,
   }),
   withoutDiff: css({
-    marginBottom: theme.spacing(2),
+    marginBottom: theme.spacing(1),
   }),
 });

public/app/features/dashboard-scene/settings/version-history/DiffValues.tsx

@@ -32,6 +32,6 @@ const getStyles = (theme: GrafanaTheme2) =>
     borderRadius: theme.shape.radius.default,
     color: theme.colors.text.primary,
     fontSize: theme.typography.body.fontSize,
-    margin: `0 ${theme.spacing(0.5)}`,
+    margin: theme.spacing(0, 0.5),
-    padding: theme.spacing(0.5, 1),
+    padding: theme.spacing(0.25, 0.5),
   });

public/app/features/dashboard-scene/settings/version-history/VersionHistoryComparison.tsx

@@ -2,7 +2,7 @@ import { css, cx } from '@emotion/css';
 import React from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
-import { Button, ModalsController, CollapsableSection, HorizontalGroup, useStyles2 } from '@grafana/ui';
+import { Button, ModalsController, CollapsableSection, useStyles2, Stack, Icon, Box } from '@grafana/ui';
 
 import { DecoratedRevisionModel } from '../VersionsEditView';
 
@@ -24,56 +24,57 @@ export const VersionHistoryComparison = ({ baseInfo, newInfo, diffData, isNewLat
   const styles = useStyles2(getStyles);
 
   return (
-    <div>
+    <Stack direction="column" gap={1}>
-      <div className={styles.spacer}>
+      <Stack justifyContent="space-between" alignItems="center">
-        <HorizontalGroup justify="space-between" align="center">
+        <Stack alignItems="center">
-          <div>
+          <span className={cx(styles.versionInfo, styles.noMarginBottom)}>
-            <p className={styles.versionInfo}>
+            <strong>Version {baseInfo.version}</strong> updated by {baseInfo.createdBy} {baseInfo.ageString}
-              <strong>Version {newInfo.version}</strong> updated by {newInfo.createdBy} {newInfo.ageString} -{' '}
+            {baseInfo.message}
-              {newInfo.message}
+          </span>
-            </p>
+          <Icon name="arrow-right" size="sm" />
-            <p className={cx(styles.versionInfo, styles.noMarginBottom)}>
+          <span className={styles.versionInfo}>
-              <strong>Version {baseInfo.version}</strong> updated by {baseInfo.createdBy} {baseInfo.ageString} -{' '}
+            <strong>Version {newInfo.version}</strong> updated by {newInfo.createdBy} {newInfo.ageString}
-              {baseInfo.message}
+            {newInfo.message}
-            </p>
+          </span>
-          </div>
+        </Stack>
-          {isNewLatest && (
+        {isNewLatest && (
-            <ModalsController>
+          <ModalsController>
-              {({ showModal, hideModal }) => (
+            {({ showModal, hideModal }) => (
-                <Button
+              <Button
-                  variant="destructive"
+                variant="destructive"
-                  icon="history"
+                icon="history"
-                  onClick={() => {
+                onClick={() => {
-                    showModal(RevertDashboardModal, {
+                  showModal(RevertDashboardModal, {
-                      version: baseInfo,
+                    version: baseInfo,
-                      onRestore,
+                    onRestore,
-                      hideModal,
+                    hideModal,
-                    });
+                  });
-                  }}
+                }}
-                >
+              >
-                  Restore to version {baseInfo.version}
+                Restore to version {baseInfo.version}
-                </Button>
+              </Button>
-              )}
+            )}
-            </ModalsController>
+          </ModalsController>
-          )}
+        )}
-        </HorizontalGroup>
+      </Stack>
-      </div>
+
-      <div className={styles.spacer}>
+      {Object.entries(diff).map(([key, diffs]) => (
-        {Object.entries(diff).map(([key, diffs]) => (
+        <DiffGroup diffs={diffs} key={key} title={key} />
-          <DiffGroup diffs={diffs} key={key} title={key} />
+      ))}
-        ))}
+
-      </div>
+      <Box paddingTop={2}>
-      <CollapsableSection isOpen={false} label="View JSON Diff">
+        <CollapsableSection isOpen={false} label="View JSON Diff">
-        <DiffViewer oldValue={JSON.stringify(diffData.lhs, null, 2)} newValue={JSON.stringify(diffData.rhs, null, 2)} />
+          <DiffViewer
-      </CollapsableSection>
+            oldValue={JSON.stringify(diffData.lhs, null, 2)}
-    </div>
+            newValue={JSON.stringify(diffData.rhs, null, 2)}
+          />
+        </CollapsableSection>
+      </Box>
+    </Stack>
   );
 };
 
 const getStyles = (theme: GrafanaTheme2) => ({
-  spacer: css({
-    marginBottom: theme.spacing(4),
-  }),
   versionInfo: css({
     color: theme.colors.text.secondary,
     fontSize: theme.typography.bodySmall.fontSize,

public/app/features/dashboard-scene/settings/version-history/VersionHistoryHeader.tsx

@@ -36,6 +36,6 @@ const getStyles = (theme: GrafanaTheme2) => ({
     fontSize: theme.typography.h3.fontSize,
     display: 'flex',
     gap: theme.spacing(2),
-    marginBottom: theme.spacing(3),
+    marginBottom: theme.spacing(2),
   }),
 });

public/app/features/dashboard/components/SaveDashboard/SaveDashboardDiff.tsx

@@ -1,9 +1,7 @@
-import { css } from '@emotion/css';
 import React, { ReactElement } from 'react';
 import { useAsync } from 'react-use';
 
-import { GrafanaTheme2 } from '@grafana/data';
+import { Box, Spinner, Stack } from '@grafana/ui';
-import { Spinner, useStyles2 } from '@grafana/ui';
 import { Diffs } from 'app/features/dashboard-scene/settings/version-history/utils';
 
 import { DiffGroup } from '../../../dashboard-scene/settings/version-history/DiffGroup';
@@ -18,7 +16,6 @@ interface SaveDashboardDiffProps {
 }
 
 export const SaveDashboardDiff = ({ diff, oldValue, newValue }: SaveDashboardDiffProps) => {
-  const styles = useStyles2(getStyles);
   const loader = useAsync(async () => {
     const oldJSON = JSON.stringify(oldValue ?? {}, null, 2);
     const newJSON = JSON.stringify(newValue ?? {}, null, 2);
@@ -27,6 +24,7 @@ export const SaveDashboardDiff = ({ diff, oldValue, newValue }: SaveDashboardDif
     let schemaChange: ReactElement | undefined = undefined;
     const diffs: ReactElement[] = [];
     let count = 0;
+
     if (diff) {
       for (const [key, changes] of Object.entries(diff)) {
         // this takes a long time for large diffs (so this is async)
@@ -39,6 +37,7 @@ export const SaveDashboardDiff = ({ diff, oldValue, newValue }: SaveDashboardDif
         count += changes.length;
       }
     }
+
     return {
       schemaChange,
       diffs,
@@ -58,19 +57,14 @@ export const SaveDashboardDiff = ({ diff, oldValue, newValue }: SaveDashboardDif
   }
 
   return (
-    <div>
+    <Stack direction="column" gap={1}>
-      {value.schemaChange && <div className={styles.spacer}>{value.schemaChange}</div>}
+      {value.schemaChange && value.schemaChange}
+      {value.showDiffs && value.diffs}
 
-      {value.showDiffs && <div className={styles.spacer}>{value.diffs}</div>}
+      <Box paddingTop={2}>
-
+        <h4>Full JSON diff</h4>
-      <h4>JSON Model</h4>
+        {value.jsonView}
-      {value.jsonView}
+      </Box>
-    </div>
+    </Stack>
   );
 };
-
-const getStyles = (theme: GrafanaTheme2) => ({
-  spacer: css`
-    margin-bottom: ${theme.v1.spacing.xl};
-  `,
-});

public/app/features/dashboard/components/VersionHistory/VersionHistoryComparison.tsx

@@ -2,7 +2,7 @@ import { css, cx } from '@emotion/css';
 import React from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
-import { Button, ModalsController, CollapsableSection, HorizontalGroup, useStyles2 } from '@grafana/ui';
+import { Button, ModalsController, CollapsableSection, useStyles2, Stack, Icon, Box } from '@grafana/ui';
 import { DiffGroup } from 'app/features/dashboard-scene/settings/version-history/DiffGroup';
 import { DiffViewer } from 'app/features/dashboard-scene/settings/version-history/DiffViewer';
 import { jsonDiff } from 'app/features/dashboard-scene/settings/version-history/utils';
@@ -23,55 +23,56 @@ export const VersionHistoryComparison = ({ baseInfo, newInfo, diffData, isNewLat
   const styles = useStyles2(getStyles);
 
   return (
-    <div>
+    <Stack direction="column" gap={1}>
-      <div className={styles.spacer}>
+      <Stack justifyContent="space-between" alignItems="center">
-        <HorizontalGroup justify="space-between" align="center">
+        <Stack alignItems="center">
-          <div>
+          <span className={cx(styles.versionInfo, styles.noMarginBottom)}>
-            <p className={styles.versionInfo}>
+            <strong>Version {baseInfo.version}</strong> updated by {baseInfo.createdBy} {baseInfo.ageString}
-              <strong>Version {newInfo.version}</strong> updated by {newInfo.createdBy} {newInfo.ageString} -{' '}
+            {baseInfo.message}
-              {newInfo.message}
+          </span>
-            </p>
+          <Icon name="arrow-right" size="sm" />
-            <p className={cx(styles.versionInfo, styles.noMarginBottom)}>
+          <span className={styles.versionInfo}>
-              <strong>Version {baseInfo.version}</strong> updated by {baseInfo.createdBy} {baseInfo.ageString} -{' '}
+            <strong>Version {newInfo.version}</strong> updated by {newInfo.createdBy} {newInfo.ageString}
-              {baseInfo.message}
+            {newInfo.message}
-            </p>
+          </span>
-          </div>
+        </Stack>
-          {isNewLatest && (
+        {isNewLatest && (
-            <ModalsController>
+          <ModalsController>
-              {({ showModal, hideModal }) => (
+            {({ showModal, hideModal }) => (
-                <Button
+              <Button
-                  variant="destructive"
+                variant="destructive"
-                  icon="history"
+                icon="history"
-                  onClick={() => {
+                onClick={() => {
-                    showModal(RevertDashboardModal, {
+                  showModal(RevertDashboardModal, {
-                      version: baseInfo.version,
+                    version: baseInfo.version,
-                      hideModal,
+                    hideModal,
-                    });
+                  });
-                  }}
+                }}
-                >
+              >
-                  Restore to version {baseInfo.version}
+                Restore to version {baseInfo.version}
-                </Button>
+              </Button>
-              )}
+            )}
-            </ModalsController>
+          </ModalsController>
-          )}
+        )}
-        </HorizontalGroup>
+      </Stack>
-      </div>
+
-      <div className={styles.spacer}>
+      {Object.entries(diff).map(([key, diffs]) => (
-        {Object.entries(diff).map(([key, diffs]) => (
+        <DiffGroup diffs={diffs} key={key} title={key} />
-          <DiffGroup diffs={diffs} key={key} title={key} />
+      ))}
-        ))}
+
-      </div>
+      <Box paddingTop={2}>
-      <CollapsableSection isOpen={false} label="View JSON Diff">
+        <CollapsableSection isOpen={false} label="View JSON Diff">
-        <DiffViewer oldValue={JSON.stringify(diffData.lhs, null, 2)} newValue={JSON.stringify(diffData.rhs, null, 2)} />
+          <DiffViewer
-      </CollapsableSection>
+            oldValue={JSON.stringify(diffData.lhs, null, 2)}
-    </div>
+            newValue={JSON.stringify(diffData.rhs, null, 2)}
+          />
+        </CollapsableSection>
+      </Box>
+    </Stack>
   );
 };
 
 const getStyles = (theme: GrafanaTheme2) => ({
-  spacer: css({
-    marginBottom: theme.spacing(4),
-  }),
   versionInfo: css({
     color: theme.colors.text.secondary,
     fontSize: theme.typography.bodySmall.fontSize,