[Advanced Settings] Fix code editor field #177772
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ElenaStoeva
added
Feature:Kibana Management
|
/ci |
ElenaStoeva
added
the
Team:Kibana Management
|
Pinging @elastic/platform-deployment-management (Team:Deployment Management) |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @ElenaStoeva |
yuliacech
reviewed
Mar 4, 2024
| @@ -90,14 +90,17 @@ export const CodeEditorInput = ({ | |||
| ); | |||
|
|
|||
| const debouncedUpdateValue = useMemo(() => { | |||
| // Trigger update 1000 ms after the user stopped typing to reduce validation requests to the server | |||
| return debounce(updateValue, 1000); | |||
| // Trigger update 500 ms after the user stopped typing to reduce validation requests to the server | |||
There was a problem hiding this comment.
Is there a reason to reduce the debounce period?
There was a problem hiding this comment.
The debounce period is set to 500 for the other field types and I initially set it to 1000 for the code editor to allow more time for the user to finish typing in, but later I realised that with a longer period there is a higher risk of the user clicking on the Save/reset button before the period is over. So I decided to make it 500 as the rest of the fields.
yuliacech
reviewed
Mar 4, 2024
| await debouncedUpdateValue(newValue, onUpdate); | ||
| // Only update the value when the onChange handler is called with a different value from the current one | ||
| if (newValue !== value) { | ||
| // @ts-expect-error |
There was a problem hiding this comment.
I'm curious why there is a TS error here and if we could fix it?
There was a problem hiding this comment.
Thanks for the review @yuliacech! I see that there are two TS errors in the code editor component and I think they are caused because this component is used by both json and markdown type settings, while the getFieldInputValue function expects a parameter of a typed setting field. I can't think of an easy fix at this moment so I opened #177998 to track this work.
yuliacech
approved these changes
Mar 4, 2024
There was a problem hiding this comment.
Thanks a for patching this up, @ElenaStoeva!
Tested locally and the fix works as expected 👍
ElenaStoeva
added
the
backport:prev-minor
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Mar 5, 2024
Fixes elastic#177600 ## Summary This PR fixes the incorrect behaviour described in elastic#177600, which seems to be caused because the `onChange` handler of the code editor component is redundantly called with the current value when the "Reset to default" link or the "Save" button is clicked. This fix adds a check for whether the value passed to the `onChange` is different from the current value and only then it would update the field. (cherry picked from commit f4bb26d)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Mar 5, 2024
# Backport This will backport the following commits from `main` to `8.13`: - [[Advanced Settings] Fix code editor field (#177772)](#177772) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Elena Stoeva","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-03-05T12:02:40Z","message":"[Advanced Settings] Fix code editor field (#177772)\n\nFixes https://github.com/elastic/kibana/issues/177600\r\n\r\n## Summary\r\n\r\nThis PR fixes the incorrect behaviour described in\r\nhttps://github.com//issues/177600, which seems to be\r\ncaused because the `onChange` handler of the code editor component is\r\nredundantly called with the current value when the \"Reset to default\"\r\nlink or the \"Save\" button is clicked. This fix adds a check for whether\r\nthe value passed to the `onChange` is different from the current value\r\nand only then it would update the field.","sha":"f4bb26d0efc7e6e375b8d67fad6d3a0da00bd584","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Kibana Management","Team:Deployment Management","release_note:skip","backport:prev-minor","v8.14.0"],"title":"[Advanced Settings] Fix code editor field","number":177772,"url":"https://github.com/elastic/kibana/pull/177772","mergeCommit":{"message":"[Advanced Settings] Fix code editor field (#177772)\n\nFixes https://github.com/elastic/kibana/issues/177600\r\n\r\n## Summary\r\n\r\nThis PR fixes the incorrect behaviour described in\r\nhttps://github.com//issues/177600, which seems to be\r\ncaused because the `onChange` handler of the code editor component is\r\nredundantly called with the current value when the \"Reset to default\"\r\nlink or the \"Save\" button is clicked. This fix adds a check for whether\r\nthe value passed to the `onChange` is different from the current value\r\nand only then it would update the field.","sha":"f4bb26d0efc7e6e375b8d67fad6d3a0da00bd584"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/177772","number":177772,"mergeCommit":{"message":"[Advanced Settings] Fix code editor field (#177772)\n\nFixes https://github.com/elastic/kibana/issues/177600\r\n\r\n## Summary\r\n\r\nThis PR fixes the incorrect behaviour described in\r\nhttps://github.com//issues/177600, which seems to be\r\ncaused because the `onChange` handler of the code editor component is\r\nredundantly called with the current value when the \"Reset to default\"\r\nlink or the \"Save\" button is clicked. This fix adds a check for whether\r\nthe value passed to the `onChange` is different from the current value\r\nand only then it would update the field.","sha":"f4bb26d0efc7e6e375b8d67fad6d3a0da00bd584"}}]}] BACKPORT--> Co-authored-by: Elena Stoeva <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #177600
Summary
This PR fixes the incorrect behaviour described in #177600, which seems to be caused because the
onChangehandler of the code editor component is redundantly called with the current value when the "Reset to default" link or the "Save" button is clicked. This fix adds a check for whether the value passed to theonChangeis different from the current value and only then it would update the field.