Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Advanced Settings] Fix code editor field #177772

Merged
merged 2 commits into from
Mar 5, 2024
Merged

[Advanced Settings] Fix code editor field #177772

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8a8246
[Advanced Settings] Fix code editor field
ElenaStoeva Feb 25, 2024
585253d
Merge branch 'main' into advanced-settings/fix-code-editor-field
ElenaStoeva Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions packages/kbn-management/settings/components/field_input/input/code_editor_input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to reduce the debounce period?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return debounce(updateValue, 500);
}, [updateValue]);

const onChange: CodeEditorProps['onChange'] = async (newValue) => {
// @ts-expect-error
setValue(newValue);
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why there is a TS error here and if we could fix it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

setValue(newValue);
await debouncedUpdateValue(newValue, onUpdate);
}
};

useEffect(() => {
Expand Down