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

Execute Script V2 - UX #15461

Open
wants to merge 39 commits into
base: execute-script-v2
Choose a base branch
from

Conversation

deanhannigan
Copy link
Contributor

@deanhannigan deanhannigan commented Jan 30, 2025

Description

UX updates to accomodate the new version of the JavaScript automation type.

  • Added the UX for the new JavaScript step type (EXECUTE_SCRIPT2)
    • The code editor is now displayed directly in the step, making the code immediately visible and allowing quicker edits.
    • The editor provides the same the core builder binding UX with suggestions and snippets.
  • CodeEditorField - extracted the CodeEditor UX from deep with the BindingPanel so that it can be used as a standalone UX. Supports JS only for the moment
    • A precursor to future binding fields improvements. The field would ideally house all binding UX behaviours and replace the CodeEditor in the BindingPanel entirely. That is beyond the scope of this ticket.
  • Update Row binding UX switched back from modal to drawer mode to keep it in line with the other steps.
  • Added an automation evaluation context containing; Trigger, User, Step and Settings data. When selecting a suggestion in the dropdown, it will display any available data in the UI!
  • Legacy JavaScript (EXECUTE_SCRIPT) step has been deprecated. Any automations still using the legacy step will function as before and no action is required.

Fixes

  • Fix - Binding icons in Attachment|Signature drawers were all defaulted to ShareAndroid. They reflect the correct category now.
  • Fix - Timing issue with the Bindable drawer across the builder.
    • When the text value was updated, other elements in the drawer would update but CodeEditor would get stuck with the initiating value. This caused a timing issue for all bindable inputs if they had an "update on blur" UX.
  • Fix - Added support for using spaces when configuring fields for App Action triggers. This has been an annoyance for a while.
    • My field encodes as trigger.fields.[My field] and is displayed as trigger.fields.My field.
  • Fix - Blurring a field name input in an App Action step, without altering it, would trigger an empty update and throw an error
  • Fix - Webhooks that have no test data would throw errors when modifying.
  • Fix - Minor adjustment to the App Action trigger field UX adding a little flexibility when configuring
    • Empty/unused fields will still be cleared after a successful save
  • Fix - Added table parameter to the Row action in the test endpoint as it was missing from the test results.
  • Fix - Automation icons panel was displaying the incorrect icons. The list would display the icon associated with the first automation in the group. Now they reflect their corresponding automation definition types
  • Fix - Row types mislabelled as 'Multi select' in the bindings drawer.

Addresses

Screenshots

JavaScript V2 UX. The code is directly visible and editable in the step, with suggestions and snippets . Suggestions will also render/evaluate any data in your test results or some default data
Screenshot 2025-01-31 at 14 44 14

Live binding evaluation in all fields!
Screenshot 2025-01-31 at 14 49 54

Launchcontrol

Automation Execute Script V2 and general Automation fixes

Feature branch env

Feature Branch Link

Copy link

qa-wolf bot commented Jan 30, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@deanhannigan deanhannigan marked this pull request as ready for review February 3, 2025 10:21
@deanhannigan deanhannigan requested a review from a team as a code owner February 3, 2025 10:21
@deanhannigan deanhannigan requested review from adrinr and removed request for a team February 3, 2025 10:21
@deanhannigan deanhannigan added the feature-branch Release this PR code into a feature branch label Feb 3, 2025
@mike12345567
Copy link
Collaborator

Copy link
Member

@aptkingston aptkingston left a comment

Choose a reason for hiding this comment

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

Awesome work Dean - this is a massive improvement. It worked flawlessly and passed all my checks:

  • All bindings work
  • Helpers work
  • Snippets work
  • Full evaluation context

I've noted a few minor code things but nothing major.

One question I do have - it looks like you made changes to the existing binding panels, but then ended up making your own one (CodeEditorField)? Was it not possible to reuse the existing components for this use case? It's totally fine if not, I just noticed there's a fair bit of duplication between BindingPanel and CodeEditorField which might make maintenance harder in future.

@@ -140,6 +144,7 @@
? [hbAutocomplete([...bindingsToCompletions(bindings, codeMode)])]
: []

// TODO: check if it inputData != newInputData (memo)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if these are done and left in by accident, or still need done, so just flagging them!

@@ -156,6 +161,7 @@
}

const setDefaultEnumValues = () => {
// TODO: Update this for memoisation
Copy link
Member

Choose a reason for hiding this comment

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

TODO

@@ -71,6 +69,10 @@
let expressionError: string | undefined
let evaluating = false

// Ensure these values are not stale
$: jsValue = initialValueJS ? value : null
$: hbsValue = initialValueJS ? null : value
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little apprehensive about this change just because it's so core, but I can't see any immediate issues with it. Right now this component is intended to be in charge of it's own value after mounting, which is why none of the initial value variables here are dynamic.

I'm not actually sure how this makes a difference, because initialValueJS itself is not reactive and is only set once on mount, so I can't see how making the assignment of jsValue and hbsValue reactive would change functionality. They would re-evaluate when value changes (but they are also updated by the component itself further down on chance), probably causing a wasted double update, but they reference initialValueJS which means they could potentially incorrectly set a JS value into HBS and vice versa if the current mode is different to the initial mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aptkingston and I discussed this and decided that the more appropriate approach to this was to better manage the state on blur for the editor field, rather than make a global change to the reactivity of the base editor values.

Updated behaviour: #15545

@deanhannigan
Copy link
Contributor Author

One question I do have - it looks like you made changes to the existing binding panels, but then ended up making your own one (CodeEditorField)? Was it not possible to reuse the existing components for this use case? It's totally fine if not, I just noticed there's a fair bit of duplication between BindingPanel and CodeEditorField which might make maintenance harder in future.

Thanks again for taking a look! Yep, regarding the CodeEditorField, I wanted to extract the core functionality in isolation rather than ripping apart the BindingPanel for the moment. I want to eventually replace the core elements with the CodeEditorField but thought it would be out of scope to do that as part of this ticket. There is work in the pipeline for a bindable input field and I thought it would be better to address it then.

aptkingston and others added 4 commits February 13, 2025 12:42
Pull out new code editor field and drawer bindable slot into single component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-branch Release this PR code into a feature branch size/l
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants