mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 17:59:23 -04:00
[8.7] [Security Solution] Fix cursor jumping to end of text area when editing Rule Action Message (#150823) (#151518)
# Backport This will backport the following commits from `main` to `8.7`: - [[Security Solution] Fix cursor jumping to end of text area when editing Rule Action Message (#150823)](https://github.com/elastic/kibana/pull/150823) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Garrett Spong","email":"spong@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-02-16T21:05:26Z","message":"[Security Solution] Fix cursor jumping to end of text area when editing Rule Action Message (#150823)\n\n## Summary\r\n\r\nResolves: https://github.com/elastic/kibana/issues/149885\r\n\r\nFor additional details, please see\r\nhttps://github.com/elastic/kibana/pull/141811#discussion_r981346237 and\r\nhttps://github.com/elastic/kibana/issues/142217.\r\n\r\nAs mentioned in https://github.com/elastic/kibana/issues/142217, this\r\nissue is the result of managing stale events and timings (renderings +\r\nfield updates) between the Detections `RuleActionsField` component,\r\nvalidation within the hooks-form lib, and field updates coming from the\r\n`trigger_actions_ui` components.\r\n\r\nNote: this behavior is explicit to the Edit Rule flow (`ActionsStepRule`\r\nform), and not the Bulk Actions flyout (`RuleActionsFormData` form) as\r\nevents flow as intended in the latter, presumably because of all the\r\nnested components and use of `useFormData` within the Edit Rule flow\r\n(see [form libs\r\ndocs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).\r\n\r\nThe fix here is a modification of the fix provided in\r\nhttps://github.com/elastic/kibana/pull/141811 with `setTimeout`, but\r\ninstead of always pushing the params update to be within a `setTimeout`,\r\nit now only does it when initially loading `Actions` with empty\r\n`Params`. Since this fix also has the unintended side-effect of\r\nflickering the validation error callout when first adding an action,\r\nvalidation is now throttled to 100ms intervals, which also helps with\r\nextraneous re-renders.\r\n\r\nBefore initial fix (with cursor issue) / Before throttle fix w/ flashing\r\nerror callout:\r\n<p align=\"center\">\r\n<img width=\"350\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif\"\r\n/> <img width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif\"\r\n/>\r\n</p>\r\n\r\nAfter:\r\n<p align=\"center\">\r\n<img width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif\"\r\n/>\r\n</p>","sha":"736759af76623966684e6dbb06009f1517a8d8a8","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Detections and Resp","Team: SecuritySolution","Feature:Rule Actions","Team:Detection Rules","v8.7.0","v8.8.0"],"number":150823,"url":"https://github.com/elastic/kibana/pull/150823","mergeCommit":{"message":"[Security Solution] Fix cursor jumping to end of text area when editing Rule Action Message (#150823)\n\n## Summary\r\n\r\nResolves: https://github.com/elastic/kibana/issues/149885\r\n\r\nFor additional details, please see\r\nhttps://github.com/elastic/kibana/pull/141811#discussion_r981346237 and\r\nhttps://github.com/elastic/kibana/issues/142217.\r\n\r\nAs mentioned in https://github.com/elastic/kibana/issues/142217, this\r\nissue is the result of managing stale events and timings (renderings +\r\nfield updates) between the Detections `RuleActionsField` component,\r\nvalidation within the hooks-form lib, and field updates coming from the\r\n`trigger_actions_ui` components.\r\n\r\nNote: this behavior is explicit to the Edit Rule flow (`ActionsStepRule`\r\nform), and not the Bulk Actions flyout (`RuleActionsFormData` form) as\r\nevents flow as intended in the latter, presumably because of all the\r\nnested components and use of `useFormData` within the Edit Rule flow\r\n(see [form libs\r\ndocs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).\r\n\r\nThe fix here is a modification of the fix provided in\r\nhttps://github.com/elastic/kibana/pull/141811 with `setTimeout`, but\r\ninstead of always pushing the params update to be within a `setTimeout`,\r\nit now only does it when initially loading `Actions` with empty\r\n`Params`. Since this fix also has the unintended side-effect of\r\nflickering the validation error callout when first adding an action,\r\nvalidation is now throttled to 100ms intervals, which also helps with\r\nextraneous re-renders.\r\n\r\nBefore initial fix (with cursor issue) / Before throttle fix w/ flashing\r\nerror callout:\r\n<p align=\"center\">\r\n<img width=\"350\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif\"\r\n/> <img width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif\"\r\n/>\r\n</p>\r\n\r\nAfter:\r\n<p align=\"center\">\r\n<img width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif\"\r\n/>\r\n</p>","sha":"736759af76623966684e6dbb06009f1517a8d8a8"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/150823","number":150823,"mergeCommit":{"message":"[Security Solution] Fix cursor jumping to end of text area when editing Rule Action Message (#150823)\n\n## Summary\r\n\r\nResolves: https://github.com/elastic/kibana/issues/149885\r\n\r\nFor additional details, please see\r\nhttps://github.com/elastic/kibana/pull/141811#discussion_r981346237 and\r\nhttps://github.com/elastic/kibana/issues/142217.\r\n\r\nAs mentioned in https://github.com/elastic/kibana/issues/142217, this\r\nissue is the result of managing stale events and timings (renderings +\r\nfield updates) between the Detections `RuleActionsField` component,\r\nvalidation within the hooks-form lib, and field updates coming from the\r\n`trigger_actions_ui` components.\r\n\r\nNote: this behavior is explicit to the Edit Rule flow (`ActionsStepRule`\r\nform), and not the Bulk Actions flyout (`RuleActionsFormData` form) as\r\nevents flow as intended in the latter, presumably because of all the\r\nnested components and use of `useFormData` within the Edit Rule flow\r\n(see [form libs\r\ndocs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).\r\n\r\nThe fix here is a modification of the fix provided in\r\nhttps://github.com/elastic/kibana/pull/141811 with `setTimeout`, but\r\ninstead of always pushing the params update to be within a `setTimeout`,\r\nit now only does it when initially loading `Actions` with empty\r\n`Params`. Since this fix also has the unintended side-effect of\r\nflickering the validation error callout when first adding an action,\r\nvalidation is now throttled to 100ms intervals, which also helps with\r\nextraneous re-renders.\r\n\r\nBefore initial fix (with cursor issue) / Before throttle fix w/ flashing\r\nerror callout:\r\n<p align=\"center\">\r\n<img width=\"350\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif\"\r\n/> <img width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif\"\r\n/>\r\n</p>\r\n\r\nAfter:\r\n<p align=\"center\">\r\n<img width=\"242\"\r\nsrc=\"https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif\"\r\n/>\r\n</p>","sha":"736759af76623966684e6dbb06009f1517a8d8a8"}}]}] BACKPORT--> Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
This commit is contained in:
parent
595f230bab
commit
165b09e0bb
5 changed files with 88 additions and 25 deletions
|
@ -41,7 +41,7 @@ import {
|
|||
import { getAllActionMessageParams } from '../../../../../../detections/pages/detection_engine/rules/helpers';
|
||||
|
||||
import { RuleActionsField } from '../../../../../../detections/components/rules/rule_actions_field';
|
||||
import { validateRuleActionsField } from '../../../../../../detections/containers/detection_engine/rules/validate_rule_actions_field';
|
||||
import { debouncedValidateRuleActionsField } from '../../../../../../detections/containers/detection_engine/rules/validate_rule_actions_field';
|
||||
|
||||
const CommonUseField = getUseField({ component: Field });
|
||||
|
||||
|
@ -61,7 +61,10 @@ const getFormSchema = (
|
|||
actions: {
|
||||
validations: [
|
||||
{
|
||||
validator: validateRuleActionsField(actionTypeRegistry),
|
||||
// Debounced validator not explicitly necessary here as the `RuleActionsFormData` form doesn't exhibit the same
|
||||
// behavior as the `ActionsStepRule` form outlined in https://github.com/elastic/kibana/issues/142217, however
|
||||
// additional renders are prevented so using for consistency
|
||||
validator: debouncedValidateRuleActionsField(actionTypeRegistry),
|
||||
},
|
||||
],
|
||||
},
|
||||
|
|
|
@ -64,6 +64,10 @@ export const RuleActionsField: React.FC<Props> = ({ field, messageVariables }) =
|
|||
triggersActionsUi: { getActionForm },
|
||||
} = useKibana().services;
|
||||
|
||||
// Workaround for setAlertActionsProperty being fired with prevProps when followed by setActionIdByIndex
|
||||
// For details see: https://github.com/elastic/kibana/issues/142217
|
||||
const [isInitializingAction, setIsInitializingAction] = useState(false);
|
||||
|
||||
const actions: RuleAction[] = useMemo(
|
||||
() => (!isEmpty(field.value) ? (field.value as RuleAction[]) : []),
|
||||
[field.value]
|
||||
|
@ -83,6 +87,9 @@ export const RuleActionsField: React.FC<Props> = ({ field, messageVariables }) =
|
|||
const setActionIdByIndex = useCallback(
|
||||
(id: string, index: number) => {
|
||||
const updatedActions = [...(actions as Array<Partial<RuleAction>>)];
|
||||
if (isEmpty(updatedActions[index].params)) {
|
||||
setIsInitializingAction(true);
|
||||
}
|
||||
updatedActions[index] = deepMerge(updatedActions[index], { id });
|
||||
field.setValue(updatedActions);
|
||||
},
|
||||
|
@ -98,24 +105,30 @@ export const RuleActionsField: React.FC<Props> = ({ field, messageVariables }) =
|
|||
(key: string, value: RuleActionParam, index: number) => {
|
||||
// validation is not triggered correctly when actions params updated (more details in https://github.com/elastic/kibana/issues/142217)
|
||||
// wrapping field.setValue in setTimeout fixes the issue above
|
||||
// and triggers validation after params have been updated
|
||||
setTimeout(
|
||||
() =>
|
||||
field.setValue((prevValue: RuleAction[]) => {
|
||||
const updatedActions = [...prevValue];
|
||||
updatedActions[index] = {
|
||||
...updatedActions[index],
|
||||
params: {
|
||||
...updatedActions[index].params,
|
||||
[key]: value,
|
||||
},
|
||||
};
|
||||
return updatedActions;
|
||||
}),
|
||||
0
|
||||
);
|
||||
// and triggers validation after params have been updated, however it introduced a new issue where any additional input
|
||||
// would result in the cursor jumping to the end of the text area (https://github.com/elastic/kibana/issues/149885)
|
||||
const updateValue = () => {
|
||||
field.setValue((prevValue: RuleAction[]) => {
|
||||
const updatedActions = [...prevValue];
|
||||
updatedActions[index] = {
|
||||
...updatedActions[index],
|
||||
params: {
|
||||
...updatedActions[index].params,
|
||||
[key]: value,
|
||||
},
|
||||
};
|
||||
return updatedActions;
|
||||
});
|
||||
};
|
||||
|
||||
if (isInitializingAction) {
|
||||
setTimeout(updateValue, 0);
|
||||
setIsInitializingAction(false);
|
||||
} else {
|
||||
updateValue();
|
||||
}
|
||||
},
|
||||
[field]
|
||||
[field, isInitializingAction]
|
||||
);
|
||||
|
||||
const actionForm = useMemo(
|
||||
|
|
|
@ -8,7 +8,7 @@
|
|||
import { i18n } from '@kbn/i18n';
|
||||
|
||||
import type { ActionTypeRegistryContract } from '@kbn/triggers-actions-ui-plugin/public';
|
||||
import { validateRuleActionsField } from '../../../containers/detection_engine/rules/validate_rule_actions_field';
|
||||
import { debouncedValidateRuleActionsField } from '../../../containers/detection_engine/rules/validate_rule_actions_field';
|
||||
|
||||
import type { FormSchema } from '../../../../shared_imports';
|
||||
import type { ActionsStepRule } from '../../../pages/detection_engine/rules/types';
|
||||
|
@ -21,7 +21,9 @@ export const getSchema = ({
|
|||
actions: {
|
||||
validations: [
|
||||
{
|
||||
validator: validateRuleActionsField(actionTypeRegistry),
|
||||
// Debounced validator is necessary here to prevent error validation
|
||||
// flashing when first adding an action. Also prevents additional renders
|
||||
validator: debouncedValidateRuleActionsField(actionTypeRegistry),
|
||||
},
|
||||
],
|
||||
},
|
||||
|
|
|
@ -6,3 +6,4 @@
|
|||
*/
|
||||
|
||||
export { validateRuleActionsField } from './validate_rule_actions_field';
|
||||
export { debouncedValidateRuleActionsField } from './validate_rule_actions_field';
|
||||
|
|
|
@ -7,13 +7,22 @@
|
|||
|
||||
/* istanbul ignore file */
|
||||
|
||||
import type {
|
||||
ValidationCancelablePromise,
|
||||
ValidationFuncArg,
|
||||
ValidationResponsePromise,
|
||||
} from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib';
|
||||
import type {
|
||||
RuleAction,
|
||||
ActionTypeRegistryContract,
|
||||
} from '@kbn/triggers-actions-ui-plugin/public';
|
||||
import type { ValidationFunc, ERROR_CODE, ValidationError } from '../../../../../shared_imports';
|
||||
import type { RuleActionsFormData } from '../../../../../detection_engine/rule_management_ui/components/rules_table/bulk_actions/forms/rule_actions_form';
|
||||
import type { ActionsStepRule } from '../../../../pages/detection_engine/rules/types';
|
||||
import type { ValidationFunc, ERROR_CODE } from '../../../../../shared_imports';
|
||||
import { getActionTypeName, validateMustache, validateActionParams } from './utils';
|
||||
|
||||
export const DEFAULT_VALIDATION_TIMEOUT = 100;
|
||||
|
||||
export const validateSingleAction = async (
|
||||
actionItem: RuleAction,
|
||||
actionTypeRegistry: ActionTypeRegistryContract
|
||||
|
@ -26,9 +35,7 @@ export const validateSingleAction = async (
|
|||
|
||||
export const validateRuleActionsField =
|
||||
(actionTypeRegistry: ActionTypeRegistryContract) =>
|
||||
async (
|
||||
...data: Parameters<ValidationFunc>
|
||||
): Promise<ValidationError<ERROR_CODE> | void | undefined> => {
|
||||
async (...data: Parameters<ValidationFunc>): ValidationResponsePromise<ERROR_CODE> => {
|
||||
const [{ value, path }] = data as [{ value: RuleAction[]; path: string }];
|
||||
|
||||
const errors = [];
|
||||
|
@ -51,3 +58,40 @@ export const validateRuleActionsField =
|
|||
};
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Debounces validation by canceling previous validation requests. Essentially leveraging the async validation
|
||||
* cancellation behavior from the hook_form_lib. Necessary to prevent error validation flashing when first adding an
|
||||
* action until root cause of https://github.com/elastic/kibana/issues/142217 is found
|
||||
*
|
||||
* See docs for details:
|
||||
* https://docs.elastic.dev/form-lib/examples/validation#cancel-asynchronous-validation
|
||||
*
|
||||
* Note: _.throttle/debounce does not have async support, and so not used https://github.com/lodash/lodash/issues/4815.
|
||||
*
|
||||
* @param actionTypeRegistry
|
||||
* @param defaultValidationTimeout
|
||||
*/
|
||||
export const debouncedValidateRuleActionsField =
|
||||
(
|
||||
actionTypeRegistry: ActionTypeRegistryContract,
|
||||
defaultValidationTimeout = DEFAULT_VALIDATION_TIMEOUT
|
||||
) =>
|
||||
(data: ValidationFuncArg<ActionsStepRule | RuleActionsFormData>): ValidationResponsePromise => {
|
||||
let isCanceled = false;
|
||||
const promise: ValidationCancelablePromise = new Promise((resolve) => {
|
||||
setTimeout(() => {
|
||||
if (isCanceled) {
|
||||
resolve();
|
||||
} else {
|
||||
resolve(validateRuleActionsField(actionTypeRegistry)(data));
|
||||
}
|
||||
}, defaultValidationTimeout);
|
||||
});
|
||||
|
||||
promise.cancel = () => {
|
||||
isCanceled = true;
|
||||
};
|
||||
|
||||
return promise;
|
||||
};
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue