Fix minor Console regressions introduced during EUIfication. #41089
Conversation
- 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.
cjcenizal
added
Feature:Console
|
Pinging @elastic/es-ui |
💔 Build Failed |
| return changedSettingsAccum; | ||
| }, {}); | ||
| mappings.retrieveAutoCompleteInfo(changedSettings); | ||
| } else if (isPollingChanged && newSettings.polling) { |
There was a problem hiding this comment.
Is && newSettings.polling needed? Looks like it is already being checked on line 57.
After looking at this further, should the initial check on line 57 be removed? I think we would want to fetch any changed settings regardless if polling is turned on or off.
There was a problem hiding this comment.
My line of thought here is that users disabled polling because the fetch request is so expensive. So, I think they'd be unhappy with any fetch that's automatically executed without them clicking the "Fetch now" button. Does that make sense? I'm going to add a comment to explain that for now but I'm still open to changing this if you think we can improve it.
There was a problem hiding this comment.
Ok, makes sense. Thanks for adding the comment! I think you can still delete && newSettings.polling (my original comment). Otherwise, this is fine with me.
There was a problem hiding this comment.
isPollingChanged will also be true if the user has turned off polling. So we need to make sure we're fetching the settings only if the user has turned polling on, but not when they've turned it off.
There was a problem hiding this comment.
isn't that already being handled on line 60 though?
if (newSettings.polling) { ...
There was a problem hiding this comment.
Oh you're right! I missed that. Thank you for persisting in pointing it out to me 😅 Will fix!
| @@ -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 | |||
There was a problem hiding this comment.
Thanks for adding this!
|
Thanks for the review @alisonelizabeth! I replied to your comment and fixed the TS errors. Could you take another look? |
💚 Build Succeeded |
There was a problem hiding this comment.
Thanks for addressing my feedback @cjcenizal! Responded to your comment with one suggestion, otherwise LGTM.
| return changedSettingsAccum; | ||
| }, {}); | ||
| mappings.retrieveAutoCompleteInfo(changedSettings); | ||
| } else if (isPollingChanged && newSettings.polling) { |
There was a problem hiding this comment.
Ok, makes sense. Thanks for adding the comment! I think you can still delete && newSettings.polling (my original comment). Otherwise, this is fine with me.
💚 Build Succeeded |
…#41089) - 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.
…#41089) - 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.
…#41942) - 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.
…#41943) - 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.
This PR fixes a few regressions introduced as part of #39341.
This PR contains these changes:
Release notes
7.3.0 introduced a couple minor regressions in the UX around changing autocompletion settings, most notably that changed settings were not re-fetched immediately when the they were saved. This has been fixed.