diff --git a/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx b/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx index 814f68a400981e..68a5950269517e 100644 --- a/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx +++ b/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx @@ -70,19 +70,24 @@ export class UnlinkFromLibraryAction implements Action = { type: embeddable.type, - explicitInput: { ...newInput }, + explicitInput: { ...newInput, title: embeddable.getTitle() }, }; + // since by value visualizations should not have default titles, unlinking a visualization should remove + // the library title from the attributes. + _.unset(newPanel, 'explicitInput.attributes.title'); dashboard.replacePanel(panelToReplace, newPanel, true); const title = dashboardUnlinkFromLibraryAction.getSuccessMessage( embeddable.getTitle() ? `'${embeddable.getTitle()}'` : '' ); + this.deps.toasts.addSuccess({ title, 'data-test-subj': 'unlinkPanelSuccess', diff --git a/src/plugins/dashboard/public/application/lib/diff_dashboard_state.ts b/src/plugins/dashboard/public/application/lib/diff_dashboard_state.ts index 17b4f9f515f7a5..b64b6a24877de3 100644 --- a/src/plugins/dashboard/public/application/lib/diff_dashboard_state.ts +++ b/src/plugins/dashboard/public/application/lib/diff_dashboard_state.ts @@ -9,6 +9,7 @@ import _ from 'lodash'; import { DashboardPanelState } from '..'; import { esFilters, Filter } from '../../services/data'; +import { EmbeddableInput } from '../../services/embeddable'; import { DashboardContainerInput, DashboardOptions, @@ -64,12 +65,12 @@ export const diffDashboardState = ( }; const optionsAreEqual = (optionsA: DashboardOptions, optionsB: DashboardOptions): boolean => { - const optionKeys = [...Object.keys(optionsA), ...Object.keys(optionsB)]; + const optionKeys = [ + ...(Object.keys(optionsA) as Array), + ...(Object.keys(optionsB) as Array), + ]; for (const key of optionKeys) { - if ( - Boolean((optionsA as unknown as { [key: string]: boolean })[key]) !== - Boolean((optionsB as unknown as { [key: string]: boolean })[key]) - ) { + if (Boolean(optionsA[key]) !== Boolean(optionsB[key])) { return false; } } @@ -90,16 +91,40 @@ const panelsAreEqual = (panelsA: DashboardPanelMap, panelsB: DashboardPanelMap): const panelCommonDiff = commonDiff( panelsA[id] as unknown as DashboardDiffCommon, panelsB[id] as unknown as DashboardDiffCommon, - ['panelRefName'] + ['panelRefName', 'explicitInput'] ); - if (Object.keys(panelCommonDiff).length > 0) { + if ( + Object.keys(panelCommonDiff).length > 0 || + !explicitInputIsEqual(panelsA[id].explicitInput, panelsB[id].explicitInput) + ) { return false; } } - return true; }; +/** + * Need to compare properties of explicitInput *directly* in order to handle special comparisons for 'title' + * and 'hidePanelTitles.' For example, if some object 'obj1' has 'obj1[title] = undefined' and some other + * object `obj2' simply does not have the key `title,' we want obj1 to still equal obj2 - in normal comparisons + * without this special case, `obj1 != obj2.' + * @param originalInput + * @param newInput + */ +const explicitInputIsEqual = ( + originalInput: EmbeddableInput, + newInput: EmbeddableInput +): boolean => { + const diffs = commonDiff(originalInput, newInput, [ + 'hidePanelTitles', + 'title', + ]); + const hidePanelsAreEqual = + Boolean(originalInput.hidePanelTitles) === Boolean(newInput.hidePanelTitles); + const titlesAreEqual = originalInput.title === newInput.title; + return Object.keys(diffs).length === 0 && hidePanelsAreEqual && titlesAreEqual; +}; + const commonDiffFilters = ( originalObj: DashboardDiffCommonFilters, newObj: DashboardDiffCommonFilters, diff --git a/src/plugins/embeddable/public/lib/attribute_service/attribute_service.tsx b/src/plugins/embeddable/public/lib/attribute_service/attribute_service.tsx index e7a9127f17040e..749b968f633bd3 100644 --- a/src/plugins/embeddable/public/lib/attribute_service/attribute_service.tsx +++ b/src/plugins/embeddable/public/lib/attribute_service/attribute_service.tsx @@ -168,11 +168,11 @@ export class AttributeService< newAttributes, true )) as unknown as RefType; + // Remove unneeded attributes from the original input. Note that the original panel title + // is removed in favour of the new attributes title + const newInput = omit(input, [ATTRIBUTE_SERVICE_KEY, 'title']); - // Remove unneeded attributes from the original input. - const newInput = omit(input, ATTRIBUTE_SERVICE_KEY); - - // Combine input and wrapped input to preserve any passed in explicit Input. + // Combine input and wrapped input to preserve any passed in explicit Input resolve({ ...newInput, ...wrappedInput }); return { id: wrappedInput.savedObjectId }; } catch (error) { diff --git a/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx b/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx index 742a2d19099418..75a33b0e7336b3 100644 --- a/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx +++ b/src/plugins/embeddable/public/lib/panel/panel_header/panel_header.tsx @@ -153,7 +153,7 @@ export function PanelHeader({ if (!showPanelBar) { return ( -
+
-

- {getAriaLabel()} - {renderTitle()} - {renderBadges(badges, embeddable)} -

- {renderNotifications(notifications, embeddable)} - - + +
+

+ {getAriaLabel()} + {renderTitle()} + {renderBadges(badges, embeddable)} +

+ {renderNotifications(notifications, embeddable)} + +
+ ); } diff --git a/src/plugins/visualizations/public/vis.ts b/src/plugins/visualizations/public/vis.ts index ad5c0705d75394..fd9bb434a7cf0c 100644 --- a/src/plugins/visualizations/public/vis.ts +++ b/src/plugins/visualizations/public/vis.ts @@ -200,7 +200,7 @@ export class Vis { data: { aggs: aggs as any, searchSource: this.data.searchSource ? this.data.searchSource.getSerializedFields() : {}, - savedSearchId: this.data.savedSearchId, + ...(this.data.savedSearchId ? { savedSearchId: this.data.savedSearchId } : {}), }, }; } diff --git a/test/functional/page_objects/dashboard_page.ts b/test/functional/page_objects/dashboard_page.ts index 77ea098c76878b..c3f88d20369af1 100644 --- a/test/functional/page_objects/dashboard_page.ts +++ b/test/functional/page_objects/dashboard_page.ts @@ -534,6 +534,27 @@ export class DashboardPageObject extends FtrService { return await Promise.all(titleObjects.map(async (title) => await title.getVisibleText())); } + // returns an array of Boolean values - true if the panel title is visible in view mode, false if it is not + public async getVisibilityOfPanelTitles() { + this.log.debug('in getVisibilityOfPanels'); + // only works if the dashboard is in view mode + const inViewMode = await this.getIsInViewMode(); + if (!inViewMode) { + await this.clickCancelOutOfEditMode(); + } + const visibilities: boolean[] = []; + const titleObjects = await this.testSubjects.findAll('dashboardPanelTitle__wrapper'); + for (const titleObject of titleObjects) { + const exists = !(await titleObject.elementHasClass('embPanel__header--floater')); + visibilities.push(exists); + } + // return to edit mode if a switch to view mode above was necessary + if (!inViewMode) { + await this.switchToEditMode(); + } + return visibilities; + } + public async getPanelDimensions() { const panels = await this.find.allByCssSelector('.react-grid-item'); // These are gridster-defined elements and classes return await Promise.all( diff --git a/test/functional/services/dashboard/panel_actions.ts b/test/functional/services/dashboard/panel_actions.ts index 4340f16492a7c2..c7e786f4b123a4 100644 --- a/test/functional/services/dashboard/panel_actions.ts +++ b/test/functional/services/dashboard/panel_actions.ts @@ -316,7 +316,9 @@ export class DashboardPanelActionsService extends FtrService { } else { await this.customizePanel(); } - await this.testSubjects.setValue('customEmbeddablePanelTitleInput', customTitle); + await this.testSubjects.setValue('customEmbeddablePanelTitleInput', customTitle, { + clearWithKeyboard: customTitle === '', // if clearing the title using the empty string as the new value, 'clearWithKeyboard' must be true; otherwise, false + }); await this.testSubjects.click('saveNewTitleButton'); } diff --git a/x-pack/test/functional/apps/dashboard/index.ts b/x-pack/test/functional/apps/dashboard/index.ts index be817faeaa6c49..4aa379ac19dc72 100644 --- a/x-pack/test/functional/apps/dashboard/index.ts +++ b/x-pack/test/functional/apps/dashboard/index.ts @@ -20,6 +20,7 @@ export default function ({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./dashboard_tagging')); loadTestFile(require.resolve('./dashboard_lens_by_value')); loadTestFile(require.resolve('./dashboard_maps_by_value')); + loadTestFile(require.resolve('./panel_titles')); loadTestFile(require.resolve('./migration_smoke_tests/lens_migration_smoke_test')); loadTestFile(require.resolve('./migration_smoke_tests/controls_migration_smoke_test')); diff --git a/x-pack/test/functional/apps/dashboard/panel_titles.ts b/x-pack/test/functional/apps/dashboard/panel_titles.ts new file mode 100644 index 00000000000000..7cca12b4ee4370 --- /dev/null +++ b/x-pack/test/functional/apps/dashboard/panel_titles.ts @@ -0,0 +1,158 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const esArchiver = getService('esArchiver'); + const kibanaServer = getService('kibanaServer'); + const testSubjects = getService('testSubjects'); + const retry = getService('retry'); + const dashboardPanelActions = getService('dashboardPanelActions'); + const PageObjects = getPageObjects([ + 'common', + 'dashboard', + 'visualize', + 'visEditor', + 'timePicker', + 'lens', + ]); + + const DASHBOARD_NAME = 'Panel Title Test'; + const CUSTOM_TITLE = 'Test Custom Title'; + const EMPTY_TITLE = '[No Title]'; + const LIBRARY_TITLE_FOR_CUSTOM_TESTS = 'Library Title for Custom Title Tests'; + const LIBRARY_TITLE_FOR_EMPTY_TESTS = 'Library Title for Empty Title Tests'; + + describe('panel titles', () => { + before(async () => { + await esArchiver.load('test/functional/fixtures/es_archiver/dashboard/current/kibana'); + await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/logstash_functional'); + await kibanaServer.importExport.load( + 'x-pack/test/functional/fixtures/kbn_archiver/lens/lens_basic.json' + ); + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.preserveCrossAppState(); + await PageObjects.dashboard.clickNewDashboard(); + await PageObjects.dashboard.saveDashboard(DASHBOARD_NAME); + }); + + describe('panel titles - by value', () => { + const clearUnsavedChanges = async () => { + await retry.try(async () => { + // avoid flaky test by surrounding in retry + await testSubjects.existOrFail('dashboardUnsavedChangesBadge'); + await PageObjects.dashboard.clickQuickSave(); + await testSubjects.missingOrFail('dashboardUnsavedChangesBadge'); + }); + }; + + it('new panel by value has empty title', async () => { + await PageObjects.lens.createAndAddLensFromDashboard({}); + const newPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(newPanelTitle).to.equal(EMPTY_TITLE); + }); + + it('saving new panel with blank title clears "unsaved changes" badge', async () => { + await dashboardPanelActions.setCustomPanelTitle(''); + await clearUnsavedChanges(); + }); + + it('custom title causes unsaved changes and saving clears it', async () => { + await dashboardPanelActions.setCustomPanelTitle(CUSTOM_TITLE); + const panelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(panelTitle).to.equal(CUSTOM_TITLE); + await clearUnsavedChanges(); + }); + + it('resetting title on a by value panel sets it to the empty string', async () => { + const BY_VALUE_TITLE = 'Reset Title - By Value'; + await dashboardPanelActions.setCustomPanelTitle(BY_VALUE_TITLE); + + await dashboardPanelActions.resetCustomPanelTitle(); + const panelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(panelTitle).to.equal(EMPTY_TITLE); + await clearUnsavedChanges(); + }); + + it('blank titles are hidden in view mode', async () => { + await PageObjects.dashboard.clickCancelOutOfEditMode(); + + const titleVisibility = (await PageObjects.dashboard.getVisibilityOfPanelTitles())[0]; + expect(titleVisibility).to.be(false); + }); + + it('custom titles are visible in view mode', async () => { + await PageObjects.dashboard.switchToEditMode(); + await dashboardPanelActions.setCustomPanelTitle(CUSTOM_TITLE); + await PageObjects.dashboard.clickQuickSave(); + await PageObjects.dashboard.clickCancelOutOfEditMode(); + + const titleVisibility = (await PageObjects.dashboard.getVisibilityOfPanelTitles())[0]; + expect(titleVisibility).to.be(true); + }); + + it('hiding an individual panel title hides it in view mode', async () => { + await PageObjects.dashboard.switchToEditMode(); + await dashboardPanelActions.toggleHidePanelTitle(); + await PageObjects.dashboard.clickQuickSave(); + await PageObjects.dashboard.clickCancelOutOfEditMode(); + + const titleVisibility = (await PageObjects.dashboard.getVisibilityOfPanelTitles())[0]; + expect(titleVisibility).to.be(false); + + // undo the previous hide panel toggle (i.e. make the panel visible) to keep state consistent + await PageObjects.dashboard.switchToEditMode(); + await dashboardPanelActions.toggleHidePanelTitle(); + await PageObjects.dashboard.clickQuickSave(); + }); + }); + + describe('panel titles - by reference', () => { + it('linking a by value panel with a custom title to the library will overwrite the custom title with the library title', async () => { + await dashboardPanelActions.setCustomPanelTitle(CUSTOM_TITLE); + await dashboardPanelActions.saveToLibrary(LIBRARY_TITLE_FOR_CUSTOM_TESTS); + await retry.try(async () => { + // need to surround in 'retry' due to delays in HTML updates causing the title read to be behind + const newPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(newPanelTitle).to.equal(LIBRARY_TITLE_FOR_CUSTOM_TESTS); + }); + }); + + it('resetting title on a by reference panel sets it to the library title', async () => { + await dashboardPanelActions.setCustomPanelTitle('This should go away'); + await dashboardPanelActions.resetCustomPanelTitle(); + const resetPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(resetPanelTitle).to.equal(LIBRARY_TITLE_FOR_CUSTOM_TESTS); + }); + + it('unlinking a by reference panel with a custom title will keep the current title', async () => { + await dashboardPanelActions.setCustomPanelTitle(CUSTOM_TITLE); + await dashboardPanelActions.unlinkFromLibary(); + const newPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(newPanelTitle).to.equal(CUSTOM_TITLE); + }); + + it("linking a by value panel with a blank title to the library will set the panel's title to the library title", async () => { + await dashboardPanelActions.setCustomPanelTitle(''); + await dashboardPanelActions.saveToLibrary(LIBRARY_TITLE_FOR_EMPTY_TESTS); + await retry.try(async () => { + // need to surround in 'retry' due to delays in HTML updates causing the title read to be behind + const newPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(newPanelTitle).to.equal(LIBRARY_TITLE_FOR_EMPTY_TESTS); + }); + }); + + it('unlinking a by reference panel without a custom title will keep the library title', async () => { + await dashboardPanelActions.unlinkFromLibary(); + const newPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0]; + expect(newPanelTitle).to.equal(LIBRARY_TITLE_FOR_EMPTY_TESTS); + }); + }); + }); +}