Fixed adding an extra space character on selecting alert variable in action text fields

[YulNaumenko]

Resolve #68208

New variables button and behavior:

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[gmmorris]

Made a comment for the email fix, but I think it's true for all of them.

[pmuellr]

Concur w/Gidi that there seems to be something off here.

Trying it out locally, seems to set the field to the "{{variable}}" (no blanks) if the current field is empty, but if there's any content in the field, then sets it to " {{variable}}" (one preceding blank) if the current field is non-empty.

It would actually be nice if we could just insert the {{variable}} text at the insertion point (cursor) of the text field, if it has focus, so it would be inserted where you'd expect - not sure we have access to the insertion point tho. Worse case, seems like we should append it to the existing text.

I don't think there are any cases where we'd want to insert a space anywhere here, unless we do have access to the insertion point - if we do, we could insert a space before and/or after the {{variable}} if the characters before/after the insertion point aren't whitespace, but ... we could leave that for another issue/PR :-)

[YulNaumenko]

Concur w/Gidi that there seems to be something off here.

Trying it out locally, seems to set the field to the "{{variable}}" (no blanks) if the current field is empty, but if there's any content in the field, then sets it to " {{variable}}" (one preceding blank) if the current field is non-empty.

It would actually be nice if we could just insert the {{variable}} text at the insertion point (cursor) of the text field, if it has focus, so it would be inserted where you'd expect - not sure we have access to the insertion point tho. Worse case, seems like we should append it to the existing text.

I don't think there are any cases where we'd want to insert a space anywhere here, unless we do have access to the insertion point - if we do, we could insert a space before and/or after the {{variable}} if the characters before/after the insertion point aren't whitespace, but ... we could leave that for another issue/PR :-)

I changed the approach for adding variables - introduced a components which allows to add variable by the cursor position :)

[gmmorris]

LGTM, this is a much cleaner approach than what we had before 👍

Left a few small notes, but nothing blocking

[pmuellr]

code LGTM

code works as expected except for the case of moving the cursor with the keyboard instead of mouse clicking. The text gets added to the end in that case, as we're not tracking cursor changes via keystroke. It would be nice if we didn't HAVE to track the cursor via the events, but could just request it when we're inserting the variable.

Nice cleanup with the new components!

[pmuellr]

for both the text field and text area, but not the JSON editor, onClick doesn't seem to be enough to get the cursor / insertion point, if you move the cursor with the keyboard instead of via mouse click. Eg, enter abc in a field, use the arrow key to move the cursor between the a and b, then insert a variable via mouse click. It gets placed after abc, not between a and b.

Is it possible to just get the selectionStart / selectionEnd fields only when needed (when inserting the variable), instead of trying to track it via events?

[pmuellr]

LGTM - works as expected

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: e94c0cc

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
triggers_actions_ui	104	+3	101

History

• 💔 Build #58522 failed d246c5a
• 💔 Build #58414 failed b9a0881
• 💔 Build #58122 failed 9d1ac4a
• 💔 Build #57756 failed 46946c0
• 💚 Build #57593 succeeded 6e90c98

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream