-
Notifications
You must be signed in to change notification settings - Fork 179
Conditional inputs Elsa 3 #188
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| using Elsa.Api.Client.Resources.ActivityDescriptors.Models; | ||
| using Microsoft.AspNetCore.Components; | ||
|
|
||
| namespace Elsa.Studio.Workflows.Components.WorkflowDefinitionEditor.Components.ActivityProperties; | ||
|
|
||
| public record ActivityInputDisplayModel(RenderFragment Editor); | ||
| public record ActivityInputDisplayModel(RenderFragment Editor, InputDescriptor InputDescriptor); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -64,6 +64,10 @@ public InputsTab() | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private ICollection<OutputDescriptor> OutputDescriptors { get; set; } = new List<OutputDescriptor>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private ICollection<ActivityInputDisplayModel> InputDisplayModels { get; set; } = new List<ActivityInputDisplayModel>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private List<string> _selectedStates = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static Dictionary<string, JsonDocument> _InputDescriptors = new(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static Dictionary<string, string> _previousStates = new(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <inheritdoc /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| protected override async Task OnParametersSetAsync() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -73,20 +77,63 @@ protected override async Task OnParametersSetAsync() | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| InputDescriptors = ActivityDescriptor.Inputs.ToList(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| OutputDescriptors = ActivityDescriptor.Outputs.ToList(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| InputDisplayModels = (await BuildInputEditorModels(Activity, ActivityDescriptor, InputDescriptors)).ToList(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| StateHasChanged(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
72
to
+80
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private async Task<IEnumerable<ActivityInputDisplayModel>> BuildInputEditorModels(JsonObject activity, ActivityDescriptor activityDescriptor, ICollection<InputDescriptor> inputDescriptors) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var models = new List<ActivityInputDisplayModel>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var browsableInputDescriptors = inputDescriptors.Where(x => x.IsBrowsable == true).ToList(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach (var inputDescriptor in browsableInputDescriptors) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach (var inputDescriptor in inputDescriptors) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| foreach (var inputDescriptor in inputDescriptors) | |
| foreach (var inputDescriptor in inputDescriptors.Where(x => x.IsBrowsable)) |
Copilot
AI
Apr 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For StateDropdown, UpdateDescription is called right after AddSelectedState, but AddSelectedState(WrappedInput) already calls UpdateDescription internally. This duplicates work and can lead to extra renders/side-effects. Remove one of the calls so description/state updates happen in a single place.
| UpdateDescription(inputDescriptor, wrappedInput); | |
| } | |
| else if (wrappedInput is null && inputDescriptor.DefaultValue is not null) | |
| { | |
| // Add the default value to the selected states | |
| AddSelectedState(inputDescriptor, inputDescriptor.DefaultValue); | |
| UpdateDescription(inputDescriptor, inputDescriptor.DefaultValue); | |
| } | |
| else if (wrappedInput is null && inputDescriptor.DefaultValue is not null) | |
| { | |
| // Add the default value to the selected states | |
| AddSelectedState(inputDescriptor, inputDescriptor.DefaultValue); |
Copilot
AI
Apr 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After parsing Description as JSON, the code assumes properties like InputType/Description/Options/Ids always exist and uses GetProperty(), which will throw and break rendering if the JSON shape differs (or if some other feature stores valid JSON in Description). Prefer validating the schema with TryGetProperty (and expected ValueKind) before reading, and treat unrecognized shapes as a standard descriptor.
Copilot
AI
Apr 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IsReadOnly no longer respects the input descriptor’s IsReadOnly flag, meaning inputs marked read-only by the server can become editable in the UI. Restore the previous logic that combines workspace read-only state with inputDescriptor.IsReadOnly.
| IsReadOnly = Workspace?.IsReadOnly ?? false | |
| IsReadOnly = (Workspace?.IsReadOnly ?? false) || inputDescriptor.IsReadOnly |
Copilot
AI
Apr 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetInputDescriptorKey concatenates ActivityDescriptor.Name and inputDescriptor.Name without a delimiter. This can produce ambiguous keys (e.g., "AB"+"C" vs "A"+"BC") and cause collisions in the descriptor cache. Add an unambiguous separator (or use a tuple-like key) to avoid collisions.
| return ActivityDescriptor?.Name + inputDescriptor.Name; | |
| var activityName = ActivityDescriptor?.Name ?? string.Empty; | |
| var inputName = inputDescriptor.Name ?? string.Empty; | |
| return $"{activityName.Length}:{activityName}|{inputName.Length}:{inputName}"; |
Copilot
AI
Apr 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AddSelectedState iterates JsonElement arrays via LINQ Count()/ElementAt() on EnumerateArray(), which repeatedly enumerates and can become O(n^2). Consider materializing to an array/list once (or iterating with a manual index) and also validate that Options and Ids lengths match before indexing to avoid mismatches.
| var stateNames = InputDescriptor.RootElement.GetProperty("Options").EnumerateArray(); | |
| var stateIds = InputDescriptor.RootElement.GetProperty("Ids").EnumerateArray(); | |
| for (var i = 0; i < stateNames.Count(); ++i) | |
| { | |
| var current = stateNames.ElementAt(i); | |
| if (current.GetString() == valueAsString) | |
| { | |
| var id = stateIds.ElementAt(i).GetString()!; | |
| var stateNamesElement = InputDescriptor.RootElement.GetProperty("Options"); | |
| var stateIdsElement = InputDescriptor.RootElement.GetProperty("Ids"); | |
| if (stateNamesElement.ValueKind != JsonValueKind.Array || stateIdsElement.ValueKind != JsonValueKind.Array) | |
| { | |
| StateHasChanged(); | |
| return; | |
| } | |
| var stateNamesCount = stateNamesElement.GetArrayLength(); | |
| var stateIdsCount = stateIdsElement.GetArrayLength(); | |
| if (stateNamesCount != stateIdsCount) | |
| { | |
| StateHasChanged(); | |
| return; | |
| } | |
| var stateNames = stateNamesElement.EnumerateArray(); | |
| var stateIds = stateIdsElement.EnumerateArray(); | |
| var stateNamesEnumerator = stateNames.GetEnumerator(); | |
| var stateIdsEnumerator = stateIds.GetEnumerator(); | |
| while (stateNamesEnumerator.MoveNext() && stateIdsEnumerator.MoveNext()) | |
| { | |
| var current = stateNamesEnumerator.Current; | |
| if (current.GetString() == valueAsString) | |
| { | |
| var id = stateIdsEnumerator.Current.GetString()!; |
Copilot
AI
Apr 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StateHasChanged() is called inside AddSelectedState(), which is invoked during BuildInputEditorModels() for each input. This can trigger a cascade of redundant renders during parameter setup and may cause flicker/perf issues. Avoid calling StateHasChanged in per-input setup code; instead update state and let the normal render cycle (or a single StateHasChanged after the loop) handle UI refresh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parsed JSON payloads are cached in static dictionaries. In a Blazor app this data will be shared across all component instances/users and will grow unbounded over time. Additionally, JsonDocument is IDisposable but stored without ever being disposed (and overwritten without disposing), which can cause memory leaks. Consider making these instance-scoped (non-static), using a bounded cache keyed to the current Activity/descriptor, and disposing documents when no longer needed (or deserialize into a POCO instead of keeping JsonDocument).