From d7db0b9e4543377a98cf83548d2e1e370d64830e Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Sun, 14 Jul 2019 13:20:37 -0700 Subject: [PATCH 1/3] Fix minor Console regressions introduced during EUIfication. - Snakecase help_example.txt filename. - Hide polling options when no autocomplete options are selected. - Apply className to settings modal so its width doesn't change when the polling options are hidden. - Only fetch changed autocomplete settings when modal is saved. --- .../core_plugins/console/public/_app.scss | 4 + .../public/src/components/editor_example.tsx | 2 +- .../{helpExample.txt => help_example.txt} | 0 .../public/src/components/settings_modal.tsx | 106 +++++++++++------- .../src/helpers/settings_show_modal.tsx | 22 +++- .../console/public/src/mappings.js | 10 ++ 6 files changed, 97 insertions(+), 47 deletions(-) rename src/legacy/core_plugins/console/public/src/components/{helpExample.txt => help_example.txt} (100%) diff --git a/src/legacy/core_plugins/console/public/_app.scss b/src/legacy/core_plugins/console/public/_app.scss index a694ea4f814b5..f3de2a9ee9e5b 100644 --- a/src/legacy/core_plugins/console/public/_app.scss +++ b/src/legacy/core_plugins/console/public/_app.scss @@ -61,3 +61,7 @@ z-index: $euiZLevel1 + 2; margin-top: 22px; } + +.conApp__settingsModal { + min-width: 460px; +} diff --git a/src/legacy/core_plugins/console/public/src/components/editor_example.tsx b/src/legacy/core_plugins/console/public/src/components/editor_example.tsx index c67b2a3644570..99309d7b8549c 100644 --- a/src/legacy/core_plugins/console/public/src/components/editor_example.tsx +++ b/src/legacy/core_plugins/console/public/src/components/editor_example.tsx @@ -19,7 +19,7 @@ import React, { useEffect } from 'react'; // @ts-ignore -import exampleText from 'raw-loader!./helpExample.txt'; +import exampleText from 'raw-loader!./help_example.txt'; import $ from 'jquery'; // @ts-ignore import SenseEditor from '../sense_editor/editor'; diff --git a/src/legacy/core_plugins/console/public/src/components/helpExample.txt b/src/legacy/core_plugins/console/public/src/components/help_example.txt similarity index 100% rename from src/legacy/core_plugins/console/public/src/components/helpExample.txt rename to src/legacy/core_plugins/console/public/src/components/help_example.txt diff --git a/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx b/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx index ac540f18bff07..1195ea06f40af 100644 --- a/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx +++ b/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useState } from 'react'; +import React, { Fragment, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; @@ -106,9 +106,70 @@ export function DevToolsSettingsModal(props: Props) { }); } + // It only makes sense to show polling options if the user needs to fetch any data. + const pollingFields = + fields || indices || templates ? ( + + + } + helpText={ + + } + > + + } + onChange={e => setPolling(e.target.checked)} + /> + + + { + // Only refresh the currently selected settings. + props.refreshAutocompleteSettings({ + autocomplete: { + fields, + indices, + templates, + }, + }); + }} + > + + + + ) : ( + undefined + ); + return ( - + + setTripleQuotes(e.target.checked)} /> + - - } - helpText={ - - } - > - - } - onChange={e => setPolling(e.target.checked)} - /> - - - - + {pollingFields} diff --git a/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx b/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx index a88955c0ca8bf..e1d8d74b59c71 100644 --- a/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx +++ b/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx @@ -32,8 +32,8 @@ export function showSettingsModal() { const container = document.getElementById('consoleSettingsModal'); const curSettings = getCurrentSettings(); - const refreshAutocompleteSettings = () => { - mappings.retrieveAutoCompleteInfo(); + const refreshAutocompleteSettings = selectedSettings => { + mappings.retrieveAutoCompleteInfo(selectedSettings); }; const closeModal = () => { @@ -54,12 +54,22 @@ export function showSettingsModal() { prevSettings: DevToolsSettings ) => { // We'll only retrieve settings if polling is on. - const isPollingChanged = prevSettings.polling !== newSettings.polling; if (newSettings.polling) { const autocompleteDiff = getAutocompleteDiff(newSettings, prevSettings); - if (autocompleteDiff.length > 0) { - mappings.retrieveAutoCompleteInfo(newSettings.autocomplete); - } else if (isPollingChanged) { + + const isSettingsChanged = autocompleteDiff.length > 0; + const isPollingChanged = prevSettings.polling !== newSettings.polling; + + if (isSettingsChanged) { + // If the user has changed one of the autocomplete settings, then we'll fetch just the + // ones which have changed. + const changedSettings = autocompleteDiff.reduce((changedSettingsAccum, setting) => { + changedSettingsAccum[setting] = newSettings.autocomplete[setting]; + return changedSettingsAccum; + }, {}); + mappings.retrieveAutoCompleteInfo(changedSettings); + } else if (isPollingChanged && newSettings.polling) { + // If the user has turned polling on, then we'll fetch all selected autocomplete settings. mappings.retrieveAutoCompleteInfo(); } } diff --git a/src/legacy/core_plugins/console/public/src/mappings.js b/src/legacy/core_plugins/console/public/src/mappings.js index 36e8d04291f62..3e223dbd99ca8 100644 --- a/src/legacy/core_plugins/console/public/src/mappings.js +++ b/src/legacy/core_plugins/console/public/src/mappings.js @@ -287,6 +287,16 @@ function retrieveSettings(settingsKey, settingsToRetrieve) { } // Retrieve all selected settings by default. +// TODO: We should refactor this to be easier to consume. Ideally this function should retrieve +// whatever settings are specified, otherwise just use the saved settings. This requires changing +// the behavior to not *clear* whatever settings have been unselected, but it's hard to tell if +// this is possible without altering the autocomplete behavior. These are the scenarios we need to +// support: +// 1. Manual refresh. Specify what we want. Fetch specified, leave unspecified alone. +// 2. Changed selection and saved: Specify what we want. Fetch changed and selected, leave +// unchanged alone (both selected and unselected). +// 3. Poll: Use saved. Fetch selected. Ignore unselected. + function retrieveAutoCompleteInfo(settingsToRetrieve = settings.getAutocomplete()) { if (pollTimeoutId) { clearTimeout(pollTimeoutId); From 5d575ed47f8762db3f1ad14f7dfa0c8e85fcb448 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 24 Jul 2019 15:17:13 -0700 Subject: [PATCH 2/3] Fix TS error. --- .../public/src/components/settings_modal.tsx | 2 +- .../src/helpers/settings_show_modal.tsx | 21 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx b/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx index 1195ea06f40af..f3ec577e43b71 100644 --- a/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx +++ b/src/legacy/core_plugins/console/public/src/components/settings_modal.tsx @@ -42,7 +42,7 @@ export type AutocompleteOptions = 'fields' | 'indices' | 'templates'; interface Props { onSaveSettings: (newSettings: DevToolsSettings) => Promise; onClose: () => void; - refreshAutocompleteSettings: () => void; + refreshAutocompleteSettings: (selectedSettings: any) => void; settings: DevToolsSettings; } diff --git a/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx b/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx index e1d8d74b59c71..1869af057162c 100644 --- a/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx +++ b/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx @@ -20,7 +20,7 @@ import { I18nContext } from 'ui/i18n'; import React from 'react'; import ReactDOM from 'react-dom'; -import { DevToolsSettingsModal } from '../components/settings_modal'; +import { DevToolsSettingsModal, AutocompleteOptions } from '../components/settings_modal'; import { DevToolsSettings } from '../components/dev_tools_settings'; // @ts-ignore @@ -32,7 +32,7 @@ export function showSettingsModal() { const container = document.getElementById('consoleSettingsModal'); const curSettings = getCurrentSettings(); - const refreshAutocompleteSettings = selectedSettings => { + const refreshAutocompleteSettings = (selectedSettings: any) => { mappings.retrieveAutoCompleteInfo(selectedSettings); }; @@ -53,7 +53,10 @@ export function showSettingsModal() { newSettings: DevToolsSettings, prevSettings: DevToolsSettings ) => { - // We'll only retrieve settings if polling is on. + // We'll only retrieve settings if polling is on. The expectation here is that if the user + // disables polling it's because they want manual control over the fetch request (possibly + // because it's a very expensive request given their cluster and bandwidth). In that case, + // they would be unhappy with any request that's sent automatically. if (newSettings.polling) { const autocompleteDiff = getAutocompleteDiff(newSettings, prevSettings); @@ -63,10 +66,14 @@ export function showSettingsModal() { if (isSettingsChanged) { // If the user has changed one of the autocomplete settings, then we'll fetch just the // ones which have changed. - const changedSettings = autocompleteDiff.reduce((changedSettingsAccum, setting) => { - changedSettingsAccum[setting] = newSettings.autocomplete[setting]; - return changedSettingsAccum; - }, {}); + const changedSettings: any = autocompleteDiff.reduce( + (changedSettingsAccum: any, setting: string): any => { + changedSettingsAccum[setting] = + newSettings.autocomplete[setting as AutocompleteOptions]; + return changedSettingsAccum; + }, + {} + ); mappings.retrieveAutoCompleteInfo(changedSettings); } else if (isPollingChanged && newSettings.polling) { // If the user has turned polling on, then we'll fetch all selected autocomplete settings. From f8f03728d9b51706c782699e535c0748d425f25f Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 24 Jul 2019 18:19:32 -0700 Subject: [PATCH 3/3] Remove redundant condition. --- .../console/public/src/helpers/settings_show_modal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx b/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx index 1869af057162c..c4f36d836dfda 100644 --- a/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx +++ b/src/legacy/core_plugins/console/public/src/helpers/settings_show_modal.tsx @@ -75,7 +75,7 @@ export function showSettingsModal() { {} ); mappings.retrieveAutoCompleteInfo(changedSettings); - } else if (isPollingChanged && newSettings.polling) { + } else if (isPollingChanged) { // If the user has turned polling on, then we'll fetch all selected autocomplete settings. mappings.retrieveAutoCompleteInfo(); }