Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
nickpeihl committed Jan 27, 2023
1 parent 6b8d436 commit ae7d063
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ export class CustomizePanelAction implements Action<CustomizePanelActionContext>
closed$.next(true);
handle.close();
};
const closeFlyout = () => {
close();
};

const handle = this.overlays.openFlyout(
toMountPoint(
Expand Down Expand Up @@ -101,6 +98,6 @@ export class CustomizePanelAction implements Action<CustomizePanelActionContext>
filter((mode) => mode !== ViewMode.EDIT),
take(1)
)
.subscribe({ next: closeFlyout, complete: closeFlyout });
.subscribe({ next: close, complete: close });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export const CustomizePanelEditor = (props: CustomizePanelProps) => {
);

const commonlyUsedRangesForDatePicker = props.commonlyUsedRanges
? props.commonlyUsedRanges!.map(
? props.commonlyUsedRanges.map(
({ from, to, display }: { from: string; to: string; display: string }) => {
return {
start: from,
Expand Down Expand Up @@ -246,7 +246,7 @@ export const CustomizePanelEditor = (props: CustomizePanelProps) => {
<EuiSwitch
checked={!inheritTimeRange}
data-test-subj="customizePanelShowCustomTimeRange"
id="hideTitle"
id="showCustomTimeRange"
label={
<FormattedMessage
defaultMessage="Apply custom time range to panel"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,3 @@ test('Can set title and description to an empty string', async () => {
expect(descriptionFieldAfter.props().value).toBe('');
expect(updateInput).toBeCalledWith({ description: '', title: '' });
});

// TODO
test.skip('Can set a custom time range for the panel', async () => {});
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.expectCustomizePanelSettingsFlyoutOpen();
await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_VIS_TITLE);
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await dashboardCustomizePanel.expectCustomizePanelSettingsFlyoutClosed();

await retry.try(async () => {
Expand All @@ -95,7 +95,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.expectCustomizePanelSettingsFlyoutOpen();
await dashboardCustomizePanel.clickToggleHidePanelTitle();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await dashboardCustomizePanel.expectCustomizePanelSettingsFlyoutClosed();
};
await toggleHideTitle();
Expand All @@ -114,7 +114,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await PageObjects.dashboard.switchToEditMode();
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.resetCustomPanelTitle();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await dashboardCustomizePanel.expectCustomizePanelSettingsFlyoutClosed();

await retry.try(async () => {
Expand All @@ -133,7 +133,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardPanelActions.customizePanel(el);
await dashboardCustomizePanel.expectCustomizePanelSettingsFlyoutOpen();
await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_SEARCH_TITLE);
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await dashboardCustomizePanel.expectCustomizePanelSettingsFlyoutClosed();

await retry.try(async () => {
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/dashboard/group5/share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
it('should have "panels" in app state when a panel has been modified', async () => {
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle('Test New Title');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.waitForRenderComplete();
await testSubjects.existOrFail('dashboardUnsavedChangesBadge');

Expand Down
4 changes: 2 additions & 2 deletions test/functional/services/dashboard/panel_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ export function DashboardCustomizePanelProvider({ getService }: FtrProviderConte
await testSubjects.click('resetCustomEmbeddablePanelDescriptionButton');
}

public async clickFlyoutPrimaryButton() {
log.debug('clickFlyoutPrimaryButton');
public async clickSaveButton() {
log.debug('clickSaveButton');
await testSubjects.click('saveCustomizePanelButton');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardCustomizePanel.clickToggleShowCustomTimeRange();
await dashboardCustomizePanel.clickToggleQuickMenuButton();
await dashboardCustomizePanel.clickCommonlyUsedTimeRange('Last_30 days');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.waitForRenderComplete();
await dashboardBadgeActions.expectExistsTimeRangeBadgeAction();
expect(await testSubjects.exists('emptyPlaceholder'));
Expand All @@ -57,7 +57,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
it('can remove a custom time range from a panel', async () => {
await dashboardBadgeActions.clickTimeRangeBadgeAction();
await dashboardCustomizePanel.clickToggleShowCustomTimeRange();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.waitForRenderComplete();
await dashboardBadgeActions.expectMissingTimeRangeBadgeAction();
expect(await testSubjects.exists('xyVisChart'));
Expand All @@ -71,7 +71,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardCustomizePanel.clickToggleShowCustomTimeRange();
await dashboardCustomizePanel.clickToggleQuickMenuButton();
await dashboardCustomizePanel.clickCommonlyUsedTimeRange('Last_30 days');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.waitForRenderComplete();
await dashboardBadgeActions.expectExistsTimeRangeBadgeAction();
expect(await testSubjects.exists('emptyPlaceholder'));
Expand All @@ -81,7 +81,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
it('can remove a custom time range from a panel', async () => {
await dashboardBadgeActions.clickTimeRangeBadgeAction();
await dashboardCustomizePanel.clickToggleShowCustomTimeRange();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.waitForRenderComplete();
await dashboardBadgeActions.expectMissingTimeRangeBadgeAction();
expect(await testSubjects.exists('xyVisChart'));
Expand Down
24 changes: 12 additions & 12 deletions x-pack/test/functional/apps/dashboard/group2/panel_titles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
it('saving new panel with blank title clears "unsaved changes" badge', async () => {
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle('');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.clearUnsavedChanges();
});

it('custom title causes unsaved changes and saving clears it', async () => {
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_TITLE);
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
const panelTitle = (await PageObjects.dashboard.getPanelTitles())[0];
expect(panelTitle).to.equal(CUSTOM_TITLE);
await PageObjects.dashboard.clearUnsavedChanges();
Expand All @@ -69,11 +69,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const BY_VALUE_TITLE = 'Reset Title - By Value';
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle(BY_VALUE_TITLE);
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();

await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.resetCustomPanelTitle();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
const panelTitle = (await PageObjects.dashboard.getPanelTitles())[0];
expect(panelTitle).to.equal(EMPTY_TITLE);
await PageObjects.dashboard.clearUnsavedChanges();
Expand All @@ -90,7 +90,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await PageObjects.dashboard.switchToEditMode();
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_TITLE);
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.clickQuickSave();
await PageObjects.dashboard.clickCancelOutOfEditMode();

Expand All @@ -102,7 +102,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await PageObjects.dashboard.switchToEditMode();
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.clickToggleHidePanelTitle();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.clickQuickSave();
await PageObjects.dashboard.clickCancelOutOfEditMode();

Expand All @@ -113,7 +113,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await PageObjects.dashboard.switchToEditMode();
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.clickToggleHidePanelTitle();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await PageObjects.dashboard.clickQuickSave();
});
});
Expand All @@ -122,7 +122,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
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.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_TITLE);
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
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
Expand All @@ -134,19 +134,19 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
it('resetting title on a by reference panel sets it to the library title', async () => {
await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle('This should go away');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();

await dashboardPanelActions.customizePanel();
await dashboardCustomizePanel.resetCustomPanelTitle();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
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.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_TITLE);
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await dashboardPanelActions.unlinkFromLibary();
const newPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0];
expect(newPanelTitle).to.equal(CUSTOM_TITLE);
Expand All @@ -155,7 +155,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
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.customizePanel();
await dashboardCustomizePanel.setCustomPanelTitle('');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

await panelActions.customizePanel();
await dashboardCustomizePanel.clickToggleShowCustomTimeRange();
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();
await dashboard.saveDashboard('Dashboard with Pie Chart');
});

Expand Down Expand Up @@ -83,7 +83,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardCustomizePanel.clickToggleShowCustomTimeRange();
await dashboardCustomizePanel.clickToggleQuickMenuButton();
await dashboardCustomizePanel.clickCommonlyUsedTimeRange('Last_90 days');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();

await dashboard.saveDashboard('Dashboard with Pie Chart');

Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/functional/apps/discover/saved_searches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboardCustomizePanel.clickToggleShowCustomTimeRange();
await dashboardCustomizePanel.clickToggleQuickMenuButton();
await dashboardCustomizePanel.clickCommonlyUsedTimeRange('Last_90 days');
await dashboardCustomizePanel.clickFlyoutPrimaryButton();
await dashboardCustomizePanel.clickSaveButton();

await PageObjects.header.waitUntilLoadingHasFinished();
expect(await dataGrid.hasNoResults()).to.be(true);
Expand Down

0 comments on commit ae7d063

Please sign in to comment.