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.

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

[Security Solution] Editing the Action Message of a detection rule results in cursor jumping to end #149885

Closed
spong opened this issue Jan 30, 2023 · 2 comments · Fixed by #150823
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Rule Actions Security Solution Rule Actions feature Feature:Rule Creation Security Solution Detection Rule Creation impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. sdh-linked Team:Detection Alerts Security Detection Alerts Area Team Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.7.0

Comments

@spong
Copy link
Member

spong commented Jan 30, 2023

Reported in 8.5, when editing a Detection Rule's Action Message, the cursor will jump to the end of the text area, preventing the user from making in-line edits.

Steps to reproduce:

  1. Edit a Detection Rule
  2. Go to Actions, then select a frequency and connector
  3. Give focus to the Message text area, add a chunk of text, move the cursor up a few lines and then start typing again
  4. Notice that after the first typed character the remaining text is appended to the end instead of where the cursor was.

Looks like this was initially introduced in 8.5 as part of this fix for managing param validation.

// 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
);

If you remove the setTimeout the Message EuiTextArea begins to function as expected. Probable conflict with onChange/onFocus set on the underlying EuiTextArea:

onChange={(e: React.ChangeEvent<HTMLTextAreaElement>) => onChangeWithMessageVariable(e)}
onFocus={(e: React.FocusEvent<HTMLTextAreaElement>) => {
setCurrentTextElement(e.target);
}}


Edit:

Looks like there's some excessive re-render going on here as well...

@spong spong added bug Fixes for quality problems that affect the customer experience triage_needed impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Team:Detection Alerts Security Detection Alerts Area Team labels Jan 30, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@banderror banderror added Team:Detections and Resp Security Detection Response Team Feature:Rule Actions Security Solution Rule Actions feature Feature:Rule Creation Security Solution Detection Rule Creation 8.7 candidate and removed triage_needed labels Jan 31, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@spong spong self-assigned this Feb 9, 2023
@spong spong removed the v8.6.2 label Feb 14, 2023
spong added a commit that referenced this issue Feb 16, 2023
…ng Rule Action Message (#150823)

## Summary

Resolves: #149885

For additional details, please see
#141811 (comment) and
#142217.

As mentioned in #142217, this
issue is the result of managing stale events and timings (renderings +
field updates) between the Detections `RuleActionsField` component,
validation within the hooks-form lib, and field updates coming from the
`trigger_actions_ui` components.

Note: this behavior is explicit to the Edit Rule flow (`ActionsStepRule`
form), and not the Bulk Actions flyout (`RuleActionsFormData` form) as
events flow as intended in the latter, presumably because of all the
nested components and use of `useFormData` within the Edit Rule flow
(see [form libs
docs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).

The fix here is a modification of the fix provided in
#141811 with `setTimeout`, but
instead of always pushing the params update to be within a `setTimeout`,
it now only does it when initially loading `Actions` with empty
`Params`. Since this fix also has the unintended side-effect of
flickering the validation error callout when first adding an action,
validation is now throttled to 100ms intervals, which also helps with
extraneous re-renders.

Before initial fix (with cursor issue) / Before throttle fix w/ flashing
error callout:
<p align="center">
<img width="350"
src="https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif"
/> <img width="242"
src="https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif"
/>
</p>

After:
<p align="center">
<img width="242"
src="https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif"
/>
</p>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Feb 16, 2023
…ng Rule Action Message (elastic#150823)

## Summary

Resolves: elastic#149885

For additional details, please see
elastic#141811 (comment) and
elastic#142217.

As mentioned in elastic#142217, this
issue is the result of managing stale events and timings (renderings +
field updates) between the Detections `RuleActionsField` component,
validation within the hooks-form lib, and field updates coming from the
`trigger_actions_ui` components.

Note: this behavior is explicit to the Edit Rule flow (`ActionsStepRule`
form), and not the Bulk Actions flyout (`RuleActionsFormData` form) as
events flow as intended in the latter, presumably because of all the
nested components and use of `useFormData` within the Edit Rule flow
(see [form libs
docs](https://docs.elastic.dev/form-lib/examples/listening-to-changes#forward-the-form-state-to-a-parent-component)).

The fix here is a modification of the fix provided in
elastic#141811 with `setTimeout`, but
instead of always pushing the params update to be within a `setTimeout`,
it now only does it when initially loading `Actions` with empty
`Params`. Since this fix also has the unintended side-effect of
flickering the validation error callout when first adding an action,
validation is now throttled to 100ms intervals, which also helps with
extraneous re-renders.

Before initial fix (with cursor issue) / Before throttle fix w/ flashing
error callout:
<p align="center">
<img width="350"
src="https://user-images.githubusercontent.com/2946766/215585424-8c056440-5906-4168-b997-aea3690c7ea5.gif"
/> <img width="242"
src="https://user-images.githubusercontent.com/2946766/217961284-704013b7-2f0a-4a24-95a4-4183348448f4.gif"
/>
</p>

After:
<p align="center">
<img width="242"
src="https://user-images.githubusercontent.com/2946766/218591986-e52456e0-e374-4d8e-bc84-9fb6da69f1f9.gif"
/>
</p>

(cherry picked from commit 736759a)
kibanamachine added a commit that referenced this issue Feb 16, 2023
… 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)](#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:
#149885 additional
details, please
see\r\nhttps://github.com//pull/141811#discussion_r981346237
and\r\nhttps://github.com//issues/142217.\r\n\r\nAs
mentioned in #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//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":"#150823
Solution] Fix cursor jumping to end of text area when editing Rule
Action Message (#150823)\n\n## Summary\r\n\r\nResolves:
#149885 additional
details, please
see\r\nhttps://github.com//pull/141811#discussion_r981346237
and\r\nhttps://github.com//issues/142217.\r\n\r\nAs
mentioned in #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//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:
#149885 additional
details, please
see\r\nhttps://github.com//pull/141811#discussion_r981346237
and\r\nhttps://github.com//issues/142217.\r\n\r\nAs
mentioned in #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//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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Rule Actions Security Solution Rule Actions feature Feature:Rule Creation Security Solution Detection Rule Creation impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. sdh-linked Team:Detection Alerts Security Detection Alerts Area Team Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.7.0
Projects
None yet
3 participants