NewPanelEditor: Save dashboard from edit mode now works, and other fixes (#23668)

f30074c7 · Torkel Ödegaard · GitHub · 165a0471 · f30074c7 · f30074c7
Commit f30074c7 authored Apr 18, 2020 by Torkel Ödegaard Committed by GitHub Apr 18, 2020
13 changed files
--- a/packages/grafana-ui/src/components/Button/Button.tsx
+++ b/packages/grafana-ui/src/components/Button/Button.tsx
@@ -67,13 +67,14 @@ const getPropertiesForVariant = (theme: GrafanaTheme, variant: ButtonVariant) =>
 export interface StyleProps {
  theme: GrafanaTheme;
  size: ComponentSize;
-  icon?: IconName;
  variant: ButtonVariant;
-  textAndIcon?: boolean;
+  hasIcon: boolean;
+  hasText: boolean;
 }
-export const getButtonStyles = stylesFactory(({ theme, size, variant, icon }: StyleProps) => {
+export const getButtonStyles = stylesFactory((props: StyleProps) => {
-  const { padding, fontSize, height } = getPropertiesForButtonSize(theme, size, icon);
+  const { theme, variant } = props;
+  const { padding, fontSize, height } = getPropertiesForButtonSize(props);
  const { background, borderColor, variantStyles } = getPropertiesForVariant(theme, variant);
  return {
@@ -105,9 +106,6 @@ export const getButtonStyles = stylesFactory(({ theme, size, variant, icon }: St
        ${variantStyles}
      `
    ),
-    buttonWithIcon: css`
-      padding-left: ${theme.spacing.sm};
-    `,
    // used for buttons with icon only
    iconButton: css`
      padding-right: 0;
@@ -139,7 +137,8 @@ export const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
      theme,
      size: otherProps.size || 'md',
      variant: variant || 'primary',
-      icon,
+      hasText: children !== undefined,
+      hasIcon: icon !== undefined,
    });
    return (
@@ -162,7 +161,8 @@ export const LinkButton = React.forwardRef<HTMLAnchorElement, ButtonLinkProps>(
      theme,
      size: otherProps.size || 'md',
      variant: variant || 'primary',
-      icon,
+      hasText: children !== undefined,
+      hasIcon: icon !== undefined,
    });
    return (

--- a/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx
+++ b/packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx
@@ -17,10 +17,16 @@ export interface RadioButtonProps {
 }
 const getRadioButtonStyles = stylesFactory((theme: GrafanaTheme, size: RadioButtonSize, fullWidth?: boolean) => {
-  const { fontSize, height } = getPropertiesForButtonSize(theme, size);
+  const { fontSize, height } = getPropertiesForButtonSize({
+    theme,
+    size,
+    hasIcon: false,
+    hasText: true,
+    variant: 'secondary',
+  });
  const horizontalPadding = theme.spacing[size] ?? theme.spacing.md;
  const c = theme.palette;
  const textColor = theme.colors.textSemiWeak;
  const textColorHover = theme.colors.text;
  const textColorActive = theme.isLight ? c.blue77 : c.blue95;

--- a/packages/grafana-ui/src/components/Forms/commonStyles.ts
+++ b/packages/grafana-ui/src/components/Forms/commonStyles.ts
 import { css } from 'emotion';
 import { GrafanaTheme } from '@grafana/data';
-import { ComponentSize } from '../../types/size';
+import { StyleProps } from '../Button';
-import { IconName } from '../../types';
 export const getFocusCss = (theme: GrafanaTheme) => `
  outline: 2px dotted transparent;
@@ -91,8 +90,9 @@ export const inputSizesPixels = (size: string) => {
  }
 };
-export const getPropertiesForButtonSize = (theme: GrafanaTheme, size: ComponentSize, icon?: IconName) => {
+export const getPropertiesForButtonSize = (props: StyleProps) => {
-  const { spacing, typography, height } = theme;
+  const { hasText, hasIcon, size } = props;
+  const { spacing, typography, height } = props.theme;
  switch (size) {
    case 'sm':
@@ -104,14 +104,14 @@ export const getPropertiesForButtonSize = (theme: GrafanaTheme, size: ComponentS
    case 'lg':
      return {
-        padding: `0 ${spacing.lg} 0 ${icon ? spacing.md : spacing.lg}`,
+        padding: `0 ${hasText ? spacing.lg : spacing.md} 0 ${hasIcon ? spacing.md : spacing.lg}`,
        fontSize: typography.size.lg,
        height: height.lg,
      };
    case 'md':
    default:
      return {
-        padding: `0 ${spacing.md} 0 ${icon ? spacing.sm : spacing.md}`,
+        padding: `0 ${hasText ? spacing.md : spacing.sm} 0 ${hasIcon ? spacing.sm : spacing.md}`,
        fontSize: typography.size.md,
        height: height.md,
      };

--- a/packages/grafana-ui/src/components/Forms/getFormStyles.ts
+++ b/packages/grafana-ui/src/components/Forms/getFormStyles.ts
@@ -19,6 +19,8 @@ export const getFormStyles = stylesFactory(
        theme,
        variant: options.variant,
        size: options.size,
+        hasIcon: false,
+        hasText: true,
      }),
      input: getInputStyles({ theme, invalid: options.invalid }),
      switch: getSwitchStyles(theme),

--- a/packages/grafana-ui/src/components/Modal/getModalStyles.ts
+++ b/packages/grafana-ui/src/components/Modal/getModalStyles.ts
@@ -32,6 +32,7 @@ export const getModalStyles = stylesFactory((theme: GrafanaTheme) => {
      opacity: 0.7;
    `,
    modalHeader: css`
+      label: modalHeader;
      background: ${theme.colors.bg2};
      border-bottom: 1px solid ${theme.colors.pageHeaderBorder};
      display: flex;
@@ -42,6 +43,7 @@ export const getModalStyles = stylesFactory((theme: GrafanaTheme) => {
      margin: 0 ${theme.spacing.md};
      display: flex;
      align-items: center;
+      line-height: 42px;
    `,
    modalHeaderIcon: css`
      margin-right: ${theme.spacing.md};

--- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
+++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
@@ -48,7 +48,7 @@ export class DashboardSettings extends PureComponent<Props> {
    return (
      <div className="dashboard-settings">
-        <div className="navbar navbar--shadow">
+        <div className="navbar navbar--edit">
          <div className="navbar-edit">
            <BackButton surface="body" onClick={this.onClose} />
          </div>

--- a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx
+++ b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx
@@ -17,7 +17,7 @@ import { Unsubscribable } from 'rxjs';
 import { DisplayMode, displayModes, PanelEditorTab } from './types';
 import { PanelEditorTabs } from './PanelEditorTabs';
 import { DashNavTimeControls } from '../DashNav/DashNavTimeControls';
-import { LocationState } from 'app/types';
+import { LocationState, CoreEvents } from 'app/types';
 import { calculatePanelSize } from './utils';
 import { initPanelEditor, panelEditorCleanUp, updatePanelEditorUIState } from './state/actions';
 import { PanelEditorUIState, setDiscardChanges } from './state/reducers';
@@ -29,6 +29,8 @@ import { VariableModel } from 'app/features/templating/types';
 import { getVariables } from 'app/features/variables/state/selectors';
 import { SubMenuItems } from 'app/features/dashboard/components/SubMenu/SubMenuItems';
 import { BackButton } from 'app/core/components/BackButton/BackButton';
+import { appEvents } from 'app/core/core';
+import { SaveDashboardModalProxy } from '../SaveDashboard/SaveDashboardModalProxy';
 interface OwnProps {
  dashboard: DashboardModel;
@@ -82,6 +84,17 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
    });
  };
+  onOpenDashboardSettings = () => {
+    this.props.updateLocation({ query: { editview: 'settings' }, partial: true });
+  };
+  onSaveDashboard = () => {
+    appEvents.emit(CoreEvents.showModalReact, {
+      component: SaveDashboardModalProxy,
+      props: { dashboard: this.props.dashboard },
+    });
+  };
  onChangeTab = (tab: PanelEditorTab) => {
    this.props.updateLocation({ query: { tab: tab.id }, partial: true });
  };
@@ -107,8 +120,14 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
    this.forceUpdate();
  };
-  onDragFinished = (pane: Pane, size: number) => {
+  onDragFinished = (pane: Pane, size?: number) => {
    document.body.style.cursor = 'auto';
+    // When the drag handle is just clicked size is undefined
+    if (!size) {
+      return;
+    }
    const targetPane = pane === Pane.Top ? 'topPaneSize' : 'rightPaneSize';
    const { updatePanelEditorUIState } = this.props;
    updatePanelEditorUIState({
@@ -228,12 +247,27 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
        </div>
        <div className={styles.toolbarLeft}>
          <div className={styles.toolbarItem}>
-            <Button onClick={this.onDiscard} variant="secondary">
+            <Button
-              Discard changes
+              icon="cog"
+              onClick={this.onOpenDashboardSettings}
+              variant="secondary"
+              title="Open dashboad settings"
+            />
+          </div>
+          <div className={styles.toolbarItem}>
+            <Button onClick={this.onDiscard} variant="secondary" title="Undo all changes">
+              Discard
            </Button>
          </div>
          <div className={styles.toolbarItem}>
-            <Button onClick={this.onPanelExit}>Apply</Button>
+            <Button onClick={this.onSaveDashboard} variant="secondary" title="Apply changes and save dashboard">
+              Save
+            </Button>
+          </div>
+          <div className={styles.toolbarItem}>
+            <Button onClick={this.onPanelExit} title="Apply changes and go back to dashboard">
+              Apply
+            </Button>
          </div>
        </div>
      </div>
@@ -335,7 +369,7 @@ enum Pane {
 /*
 * Styles
 */
-const getStyles = stylesFactory((theme: GrafanaTheme, props: Props) => {
+export const getStyles = stylesFactory((theme: GrafanaTheme, props: Props) => {
  const { uiState } = props;
  const handleColor = theme.palette.blue95;
  const paneSpaceing = theme.spacing.md;
@@ -361,7 +395,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme, props: Props) => {
      width: 100%;
      height: 100%;
      position: fixed;
-      z-index: ${theme.zIndex.modal};
+      z-index: ${theme.zIndex.sidemenu};
      top: 0;
      left: 0;
      right: 0;

--- a/public/app/features/dashboard/components/PanelEditor/state/reducers.ts
+++ b/public/app/features/dashboard/components/PanelEditor/state/reducers.ts
@@ -73,6 +73,7 @@ const pluginsSlice = createSlice({
      state.querySubscription = action.payload.querySubscription;
      state.initDone = true;
      state.isOpen = true;
+      state.shouldDiscardChanges = false;
    },
    setEditorPanelData: (state, action: PayloadAction<PanelData>) => {
      state.getData = () => action.payload;

--- a/public/app/features/dashboard/components/SaveDashboard/forms/SaveProvisionedDashboardForm.tsx
+++ b/public/app/features/dashboard/components/SaveDashboard/forms/SaveProvisionedDashboardForm.tsx
@@ -80,6 +80,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => {
      background: ${theme.isLight ? theme.palette.gray7 : theme.palette.black};
      padding: ${theme.spacing.sm} 0 ${theme.spacing.sm} ${theme.spacing.md};
      height: 400px;
+      width: 100%;
    `,
  };
 });
--- a/public/app/features/dashboard/state/DashboardModel.test.ts
+++ b/public/app/features/dashboard/state/DashboardModel.test.ts
@@ -71,6 +71,20 @@ describe('DashboardModel', () => {
      expect(panels.length).toBe(1);
    });
+    it('should save model in edit mode', () => {
+      const model = new DashboardModel({});
+      model.addPanel({ type: 'graph' });
+      const panel = model.initEditPanel(model.panels[0]);
+      panel.title = 'updated';
+      const saveModel = model.getSaveModelClone();
+      const savedPanel = saveModel.panels[0];
+      expect(savedPanel.title).toBe('updated');
+      expect(savedPanel.id).toBe(model.panels[0].id);
+    });
  });
  describe('row and panel manipulation', () => {

--- a/public/app/features/dashboard/state/DashboardModel.ts
+++ b/public/app/features/dashboard/state/DashboardModel.ts
@@ -180,10 +180,20 @@ export class DashboardModel {
    }
    // get panel save models
-    copy.panels = _.chain(this.panels)
+    copy.panels = this.panels
      .filter((panel: PanelModel) => panel.type !== 'add-panel')
-      .map((panel: PanelModel) => panel.getSaveModel())
+      .map((panel: PanelModel) => {
-      .value();
+        // If we save while editing we should include the panel in edit mode instead of the
+        // unmodified source panel
+        if (this.panelInEdit && this.panelInEdit.editSourceId === panel.id) {
+          const saveModel = this.panelInEdit.getSaveModel();
+          // while editing a panel we modify its id, need to restore it here
+          saveModel.id = this.panelInEdit.editSourceId;
+          return saveModel;
+        }
+        return panel.getSaveModel();
+      });
    //  sort by keys
    copy = sortByKeys(copy);

--- a/public/sass/components/_navbar.scss
+++ b/public/sass/components/_navbar.scss
@@ -15,8 +15,9 @@
    margin-left: 0;
  }
-  &--shadow {
+  &--edit {
-    box-shadow: $side-menu-shadow;
+    background: $panel-bg;
+    border-bottom: $panel-border;
  }
 }
@@ -43,11 +44,11 @@
  text-overflow: ellipsis;
  overflow: hidden;
  white-space: nowrap;
-  display: block;
+  display: flex;
+  align-items: center;
  margin: 0;
  font-size: $font-size-lg;
  min-height: $navbarHeight;
-  line-height: $navbarHeight;
  .gicon {
    top: -2px;
@@ -204,5 +205,5 @@ i.navbar-page-btn__search {
  display: flex;
  height: $navbarHeight;
  align-items: center;
-  padding-right: 13px;
+  padding-right: 16px;
 }
--- a/public/sass/components/_submenu.scss
+++ b/public/sass/components/_submenu.scss
@@ -30,6 +30,10 @@
    font-size: 75%;
    padding-left: 8px;
  }
+  .gf-form {
+    margin-bottom: 0;
+  }
 }
 .variable-value-link {