Fix reset UX for panel title and description (#182986)

## Summary Updates to the panel title flyout behavior to be more strict about changes to the title, including... - Input defaults to the actual title when empty (`""`), no longer fills with default viz `title`/`description`. - Uses the default title/description when the value is `undefined`, such that the value has never been set. - Adds a clear button to the `title` input. - `Reset` wording replaced with `Reset to default`, for `title` and `description`. - Only shows reset if there is a `default` non-empty `title`/`description` to reset to, applies mostly to by-value viz. - Changes the inspect panel `title` to always match the panel `title` and show `"[No Title]"` when empty.
elastic · May 14, 2024 · 382ee2d · 382ee2d
1 parent 5346e0d
commit 382ee2d
Show file tree

Hide file tree

Showing 22 changed files with 294 additions and 153 deletions.
diff --git a/packages/presentation/presentation_publishing/index.ts b/packages/presentation/presentation_publishing/index.ts
@@ -109,6 +109,7 @@ export {
 export {
   apiPublishesPanelDescription,
   apiPublishesWritablePanelDescription,
+  getPanelDescription,
   type PublishesPanelDescription,
   type PublishesWritablePanelDescription,
 } from './interfaces/titles/publishes_panel_description';

diff --git a/...resentation/presentation_publishing/interfaces/titles/publishes_panel_description.test.ts b/...resentation/presentation_publishing/interfaces/titles/publishes_panel_description.test.ts
@@ -0,0 +1,36 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { BehaviorSubject } from 'rxjs';
+import { getPanelDescription } from './publishes_panel_description';
+
+describe('getPanelDescription', () => {
+  test('should return default description when description is undefined', () => {
+    const api = {
+      panelDescription: new BehaviorSubject<string | undefined>(undefined),
+      defaultPanelDescription: new BehaviorSubject<string | undefined>('default description'),
+    };
+    expect(getPanelDescription(api)).toBe('default description');
+  });
+
+  test('should return empty description when description is empty string', () => {
+    const api = {
+      panelDescription: new BehaviorSubject<string | undefined>(''),
+      defaultPanelDescription: new BehaviorSubject<string | undefined>('default description'),
+    };
+    expect(getPanelDescription(api)).toBe('');
+  });
+
+  test('should return description when description is provided', () => {
+    const api = {
+      panelDescription: new BehaviorSubject<string | undefined>('custom description'),
+      defaultPanelDescription: new BehaviorSubject<string | undefined>('default description'),
+    };
+    expect(getPanelDescription(api)).toBe('custom description');
+  });
+});
diff --git a/...ges/presentation/presentation_publishing/interfaces/titles/publishes_panel_description.ts b/...ges/presentation/presentation_publishing/interfaces/titles/publishes_panel_description.ts
@@ -13,6 +13,10 @@ export interface PublishesPanelDescription {
   defaultPanelDescription?: PublishingSubject<string | undefined>;
 }
 
+export function getPanelDescription(api: Partial<PublishesPanelDescription>): string | undefined {
+  return api.panelDescription?.value ?? api.defaultPanelDescription?.value;
+}
+
 export type PublishesWritablePanelDescription = PublishesPanelDescription & {
   setPanelDescription: (newTitle: string | undefined) => void;
 };

diff --git a/...ages/presentation/presentation_publishing/interfaces/titles/publishes_panel_title.test.ts b/...ages/presentation/presentation_publishing/interfaces/titles/publishes_panel_title.test.ts
@@ -18,12 +18,12 @@ describe('getPanelTitle', () => {
     expect(getPanelTitle(api)).toBe('default title');
   });
 
-  test('should return default title when title is empty string', () => {
+  test('should return empty title when title is empty string', () => {
     const api = {
       panelTitle: new BehaviorSubject<string | undefined>(''),
       defaultPanelTitle: new BehaviorSubject<string | undefined>('default title'),
     };
-    expect(getPanelTitle(api)).toBe('default title');
+    expect(getPanelTitle(api)).toBe('');
   });
 
   test('should return title when title is provided', () => {

diff --git a/packages/presentation/presentation_publishing/interfaces/titles/publishes_panel_title.ts b/packages/presentation/presentation_publishing/interfaces/titles/publishes_panel_title.ts
@@ -15,7 +15,7 @@ export interface PublishesPanelTitle {
 }
 
 export function getPanelTitle(api: Partial<PublishesPanelTitle>): string | undefined {
-  return api.panelTitle?.value || api.defaultPanelTitle?.value;
+  return api.panelTitle?.value ?? api.defaultPanelTitle?.value;
 }
 
 export type PublishesWritablePanelTitle = PublishesPanelTitle & {

diff --git a/...ntation_panel/public/panel_actions/customize_panel_action/customize_panel_editor.test.tsx b/...ntation_panel/public/panel_actions/customize_panel_action/customize_panel_editor.test.tsx
@@ -19,19 +19,17 @@ import { CustomizePanelEditor } from './customize_panel_editor';
 
 describe('customize panel editor', () => {
   let api: CustomizePanelActionApi;
-  let setTitle: (title: string | undefined) => void;
+  let setTitle: (title?: string) => void;
   let setViewMode: (viewMode: ViewMode) => void;
-  let setDescription: (description: string | undefined) => void;
+  let setDescription: (description?: string) => void;
 
   beforeEach(() => {
     const titleSubject = new BehaviorSubject<string | undefined>(undefined);
-    setTitle = jest.fn().mockImplementation((title) => titleSubject.next(title));
+    setTitle = jest.fn((title) => titleSubject.next(title));
     const descriptionSubject = new BehaviorSubject<string | undefined>(undefined);
-    setDescription = jest
-      .fn()
-      .mockImplementation((description) => descriptionSubject.next(description));
+    setDescription = jest.fn((description) => descriptionSubject.next(description));
     const viewMode = new BehaviorSubject<ViewMode>('edit');
-    setViewMode = jest.fn().mockImplementation((nextViewMode) => viewMode.next(nextViewMode));
+    setViewMode = jest.fn((nextViewMode) => viewMode.next(nextViewMode));
 
     api = {
       viewMode,
@@ -75,27 +73,44 @@ describe('customize panel editor', () => {
       );
     });
 
-    it('Sets panel title on apply', () => {
+    it('should set panel title on apply', () => {
       renderPanelEditor();
       userEvent.type(screen.getByTestId('customEmbeddablePanelTitleInput'), 'New title');
       userEvent.click(screen.getByTestId('saveCustomizePanelButton'));
       expect(setTitle).toBeCalledWith('New title');
     });
 
+    it('should use default title when title is undefined', () => {
+      api.defaultPanelTitle = new BehaviorSubject<string | undefined>('Default title');
+      setTitle(undefined);
+      renderPanelEditor();
+      const titleInput = screen.getByTestId('customEmbeddablePanelTitleInput');
+      expect(titleInput).toHaveValue('Default title');
+    });
+
+    it('should use title even when empty string', () => {
+      api.defaultPanelTitle = new BehaviorSubject<string | undefined>('Default title');
+      setTitle('');
+      renderPanelEditor();
+      const titleInput = screen.getByTestId('customEmbeddablePanelTitleInput');
+      expect(titleInput).toHaveValue('');
+    });
+
     it('Resets panel title to default when reset button is pressed', () => {
       api.defaultPanelTitle = new BehaviorSubject<string | undefined>('Default title');
+      setTitle('Initial title');
       renderPanelEditor();
       userEvent.type(screen.getByTestId('customEmbeddablePanelTitleInput'), 'New title');
       userEvent.click(screen.getByTestId('resetCustomEmbeddablePanelTitleButton'));
       expect(screen.getByTestId('customEmbeddablePanelTitleInput')).toHaveValue('Default title');
     });
 
-    it('Reset panel title to undefined on apply', () => {
-      setTitle('very cool title');
+    it('should hide title reset when no default exists', () => {
+      api.defaultPanelTitle = new BehaviorSubject<string | undefined>(undefined);
+      setTitle('Initial title');
       renderPanelEditor();
-      userEvent.click(screen.getByTestId('resetCustomEmbeddablePanelTitleButton'));
-      userEvent.click(screen.getByTestId('saveCustomizePanelButton'));
-      expect(setTitle).toBeCalledWith(undefined);
+      userEvent.type(screen.getByTestId('customEmbeddablePanelTitleInput'), 'New title');
+      expect(screen.queryByTestId('resetCustomEmbeddablePanelTitleButton')).not.toBeInTheDocument();
     });
 
     test('title input receives focus when `focusOnTitle` is `true`', async () => {
@@ -128,7 +143,7 @@ describe('customize panel editor', () => {
       );
     });
 
-    it('Sets panel description on apply', () => {
+    it('should set panel description on apply', () => {
       renderPanelEditor();
       userEvent.type(
         screen.getByTestId('customEmbeddablePanelDescriptionInput'),
@@ -138,22 +153,47 @@ describe('customize panel editor', () => {
       expect(setDescription).toBeCalledWith('New description');
     });
 
-    it('Resets panel desription to default when reset button is pressed', () => {
+    it('should use default description when description is undefined', () => {
+      api.defaultPanelDescription = new BehaviorSubject<string | undefined>('Default description');
+      setDescription(undefined);
+      renderPanelEditor();
+      const descriptionInput = screen.getByTestId('customEmbeddablePanelDescriptionInput');
+      expect(descriptionInput).toHaveValue('Default description');
+    });
+
+    it('should use description even when empty string', () => {
       api.defaultPanelDescription = new BehaviorSubject<string | undefined>('Default description');
+      setDescription('');
       renderPanelEditor();
-      userEvent.type(screen.getByTestId('customEmbeddablePanelDescriptionInput'), 'New desription');
+      const descriptionInput = screen.getByTestId('customEmbeddablePanelDescriptionInput');
+      expect(descriptionInput).toHaveValue('');
+    });
+
+    it('Resets panel description to default when reset button is pressed', () => {
+      api.defaultPanelDescription = new BehaviorSubject<string | undefined>('Default description');
+      setDescription('Initial description');
+      renderPanelEditor();
+      userEvent.type(
+        screen.getByTestId('customEmbeddablePanelDescriptionInput'),
+        'New description'
+      );
       userEvent.click(screen.getByTestId('resetCustomEmbeddablePanelDescriptionButton'));
       expect(screen.getByTestId('customEmbeddablePanelDescriptionInput')).toHaveValue(
         'Default description'
       );
     });
 
-    it('Reset panel description to undefined on apply', () => {
-      setDescription('very cool description');
+    it('should hide description reset when no default exists', () => {
+      api.defaultPanelDescription = new BehaviorSubject<string | undefined>(undefined);
+      setDescription('Initial description');
       renderPanelEditor();
-      userEvent.click(screen.getByTestId('resetCustomEmbeddablePanelDescriptionButton'));
-      userEvent.click(screen.getByTestId('saveCustomizePanelButton'));
-      expect(setDescription).toBeCalledWith(undefined);
+      userEvent.type(
+        screen.getByTestId('customEmbeddablePanelDescriptionInput'),
+        'New description'
+      );
+      expect(
+        screen.queryByTestId('resetCustomEmbeddablePanelDescriptionButton')
+      ).not.toBeInTheDocument();
     });
   });
 

diff --git a/...presentation_panel/public/panel_actions/customize_panel_action/customize_panel_editor.tsx b/...presentation_panel/public/panel_actions/customize_panel_action/customize_panel_editor.tsx
@@ -33,6 +33,7 @@ import {
   apiPublishesTimeRange,
   apiPublishesUnifiedSearch,
   getInheritedViewMode,
+  getPanelDescription,
   getPanelTitle,
   PublishesUnifiedSearch,
 } from '@kbn/presentation-publishing';
@@ -62,10 +63,8 @@ export const CustomizePanelEditor = ({
    */
   const editMode = getInheritedViewMode(api) === 'edit';
   const [hideTitle, setHideTitle] = useState(api.hidePanelTitle?.value);
-  const [panelDescription, setPanelDescription] = useState(
-    api.panelDescription?.value ?? api.defaultPanelDescription?.value
-  );
   const [panelTitle, setPanelTitle] = useState(getPanelTitle(api));
+  const [panelDescription, setPanelDescription] = useState(getPanelDescription(api));
   const [timeRange, setTimeRange] = useState(
     api.timeRange$?.value ?? api.parentApi?.timeRange$?.value
   );
@@ -121,7 +120,6 @@ export const CustomizePanelEditor = ({
           <EuiSwitch
             checked={!hideTitle}
             data-test-subj="customEmbeddablePanelHideTitleSwitch"
-            disabled={!editMode}
             id="hideTitle"
             label={
               <FormattedMessage
@@ -140,23 +138,25 @@ export const CustomizePanelEditor = ({
             />
           }
           labelAppend={
-            <EuiButtonEmpty
-              size="xs"
-              data-test-subj="resetCustomEmbeddablePanelTitleButton"
-              onClick={() => setPanelTitle(api.defaultPanelTitle?.value)}
-              disabled={hideTitle || !editMode || api?.defaultPanelTitle?.value === panelTitle}
-              aria-label={i18n.translate(
-                'presentationPanel.action.customizePanel.flyout.optionsMenuForm.resetCustomTitleButtonAriaLabel',
-                {
-                  defaultMessage: 'Reset title',
-                }
-              )}
-            >
-              <FormattedMessage
-                id="presentationPanel.action.customizePanel.flyout.optionsMenuForm.resetCustomTitleButtonLabel"
-                defaultMessage="Reset"
-              />
-            </EuiButtonEmpty>
+            api?.defaultPanelTitle?.value && (
+              <EuiButtonEmpty
+                size="xs"
+                data-test-subj="resetCustomEmbeddablePanelTitleButton"
+                onClick={() => setPanelTitle(api.defaultPanelTitle?.value)}
+                disabled={hideTitle || panelTitle === api?.defaultPanelTitle?.value}
+                aria-label={i18n.translate(
+                  'presentationPanel.action.customizePanel.flyout.optionsMenuForm.resetCustomTitleButtonAriaLabel',
+                  {
+                    defaultMessage: 'Reset title to default',
+                  }
+                )}
+              >
+                <FormattedMessage
+                  id="presentationPanel.action.customizePanel.flyout.optionsMenuForm.resetCustomTitleButtonLabel"
+                  defaultMessage="Reset to default"
+                />
+              </EuiButtonEmpty>
+            )
           }
         >
           <EuiFieldText
@@ -166,7 +166,7 @@ export const CustomizePanelEditor = ({
             data-test-subj="customEmbeddablePanelTitleInput"
             name="title"
             type="text"
-            disabled={hideTitle || !editMode}
+            disabled={hideTitle}
             value={panelTitle ?? ''}
             onChange={(e) => setPanelTitle(e.target.value)}
             aria-label={i18n.translate(
@@ -185,23 +185,25 @@ export const CustomizePanelEditor = ({
             />
           }
           labelAppend={
-            <EuiButtonEmpty
-              size="xs"
-              data-test-subj="resetCustomEmbeddablePanelDescriptionButton"
-              onClick={() => setPanelDescription(api.defaultPanelDescription?.value)}
-              disabled={!editMode || api.defaultPanelDescription?.value === panelDescription}
-              aria-label={i18n.translate(
-                'presentationPanel.action.customizePanel.flyout.optionsMenuForm.resetCustomDescriptionButtonAriaLabel',
-                {
-                  defaultMessage: 'Reset description',
-                }
-              )}
-            >
-              <FormattedMessage
-                id="presentationPanel.action.customizePanel.modal.optionsMenuForm.resetCustomDescriptionButtonLabel"
-                defaultMessage="Reset"
-              />
-            </EuiButtonEmpty>
+            api.defaultPanelDescription?.value && (
+              <EuiButtonEmpty
+                size="xs"
+                data-test-subj="resetCustomEmbeddablePanelDescriptionButton"
+                onClick={() => setPanelDescription(api.defaultPanelDescription?.value)}
+                disabled={api.defaultPanelDescription?.value === panelDescription}
+                aria-label={i18n.translate(
+                  'presentationPanel.action.customizePanel.flyout.optionsMenuForm.resetCustomDescriptionButtonAriaLabel',
+                  {
+                    defaultMessage: 'Reset description to default',
+                  }
+                )}
+              >
+                <FormattedMessage
+                  id="presentationPanel.action.customizePanel.modal.optionsMenuForm.resetCustomDescriptionButtonLabel"
+                  defaultMessage="Reset to default"
+                />
+              </EuiButtonEmpty>
+            )
           }
         >
           <EuiTextArea

diff --git a/...gins/presentation_panel/public/panel_actions/inspect_panel_action/inspect_panel_action.ts b/...gins/presentation_panel/public/panel_actions/inspect_panel_action/inspect_panel_action.ts
@@ -59,7 +59,7 @@ export class InspectPanelAction implements Action<EmbeddableApiContext> {
     const panelTitle =
       getPanelTitle(embeddable) ||
       i18n.translate('presentationPanel.action.inspectPanel.untitledEmbeddableFilename', {
-        defaultMessage: 'untitled',
+        defaultMessage: '[No Title]',
       });
     const session = inspector.open(adapters, {
       title: panelTitle,

diff --git a/...ugins/saved_objects/public/save_modal/__snapshots__/saved_object_save_modal.test.tsx.snap b/...ugins/saved_objects/public/save_modal/__snapshots__/saved_object_save_modal.test.tsx.snap
diff --git a/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx b/src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx
@@ -214,7 +214,7 @@ export class SavedObjectSaveModal extends React.Component<Props, SaveModalState>
       >
         <EuiTextArea
           fullWidth
-          data-test-subj="viewDescription"
+          data-test-subj="savedObjectDescription"
           value={this.state.visualizationDescription}
           onChange={this.onDescriptionChange}
         />