Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive Chinese (Simplified) language support to the Rules Engine Editor through .NET resource-based localization, along with improved font handling for Chinese characters and UI culture switching capabilities.
Key changes:
- Localization infrastructure with .resx resource files for English and Chinese (zh-CN)
- CultureSwitcher component with JavaScript helpers for persisting language preferences
- Enhanced font stack with Chinese-compatible monospace fonts (JetBrains Mono, Cascadia Mono, Consolas)
- Refactored StateHasChanged calls to use InvokeAsync for thread safety
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/RulesEngineEditor/Resources/SharedResources.resx | English resource strings for all UI elements and error messages |
| src/RulesEngineEditor/Resources/SharedResources.zh-CN.resx | Chinese (Simplified) translations for all UI elements and error messages |
| src/RulesEngineEditor/Resources/SharedResources.Designer.cs | Auto-generated strongly-typed resource accessor class |
| src/RulesEngineEditor/Resources/SharedResources.cs | Empty marker class for resource namespace |
| src/RulesEngineEditor/Pages/RulesEngineEditorPage.razor | Updated UI elements to use localized resource strings |
| src/RulesEngineEditor/Pages/RulesEngineEditorPage.razor.cs | Added LocalizeError method and thread-safe state updates |
| src/RulesEngineEditor/Components/*.razor | Updated all components to use SharedResources for labels and messages |
| src/RulesEngineEditor/Components/CultureSwitcher.razor | New component for language selection with persistence |
| src/RulesEngineEditor/wwwroot/css/reeditor.css | Added web font declarations and Chinese-compatible font stack |
| src/RulesEngineEditor/wwwroot/fonts/consolas/README.txt | Instructions for optional Consolas font usage |
| demo/RulesEngineEditorWebAssembly/Program.cs | Culture initialization from localStorage on startup |
| demo/RulesEngineEditorWebAssembly/wwwroot/culture.js | JavaScript helpers for culture persistence |
| demo/RulesEngineEditorWebAssembly/wwwroot/index.html | Added culture.js script reference |
| demo/RulesEngineEditorServer/Startup.cs | Configured request localization middleware |
| demo/RulesEngineEditorServer/Shared/*.razor | Added CultureSwitcher to layouts and localized navigation |
| demo/RulesEngineEditorServer/Pages/_Host.cshtml | Added culture.js script reference |
Files not reviewed (1)
- src/RulesEngineEditor/Resources/SharedResources.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private string LocalizeMessage(string message) | ||
| { | ||
| if (string.IsNullOrEmpty(message)) return message; | ||
| if (message == "Rule was successful.") return RulesEngineEditor.Resources.SharedResources.RuleWasSuccessful; | ||
| if (message == "One or more adjust rules failed.") return RulesEngineEditor.Resources.SharedResources.OneOrMoreAdjustRulesFailed; | ||
| var m = message; | ||
| m = m.Replace("Validation failed:", RulesEngineEditor.Resources.SharedResources.ValidationFailed); | ||
| m = m.Replace("Severity: Error", RulesEngineEditor.Resources.SharedResources.SeverityError); | ||
| m = m.Replace("Atleast one of Rules or WorkflowsToInject must be not empty", RulesEngineEditor.Resources.SharedResources.RulesOrWorkflowsToInjectRequired); | ||
| m = m.Replace("Expression cannot be null or empty when RuleExpressionType is LambdaExpression", RulesEngineEditor.Resources.SharedResources.ExpressionCannotBeNullWhenLambda); | ||
| return m; | ||
| } |
There was a problem hiding this comment.
The LocalizeMessage method in RuleEditor.razor performs similar sequential string replacements with potential for interference. Additionally, both LocalizeError (in RulesEngineEditorPage.razor.cs) and LocalizeMessage (here) duplicate the same localization logic. Consider extracting this into a shared service or helper class to avoid code duplication and make maintenance easier.
| if (string.IsNullOrWhiteSpace(InputJSON)) | ||
| { | ||
| WorkflowService.Inputs = new List<InputRuleParameter>(); | ||
| WorkflowService.RuleParameters = Array.Empty<RuleParameter>(); |
There was a problem hiding this comment.
The early return when InputJSON is null or whitespace clears the inputs and rule parameters but doesn't trigger a state update. This could lead to the UI not reflecting the cleared state. Consider adding InvokeAsync(StateHasChanged) or calling an update method before the return to ensure the UI updates properly when the input is cleared.
| WorkflowService.RuleParameters = Array.Empty<RuleParameter>(); | |
| WorkflowService.RuleParameters = Array.Empty<RuleParameter>(); | |
| InvokeAsync(StateHasChanged); |
| await JS.InvokeVoidAsync("blazorCulture.set", selectedCulture); | ||
| await JS.InvokeVoidAsync("setCultureCookie", selectedCulture); |
There was a problem hiding this comment.
The CultureSwitcher component relies on two JavaScript functions (blazorCulture.set and setCultureCookie) that are defined in culture.js, but there's no error handling if these functions are not available or fail to execute. If culture.js fails to load or the functions are not defined, this will cause a runtime JavaScript exception. Consider adding try-catch around the JS invocations or checking if the functions exist before calling them.
| await JS.InvokeVoidAsync("blazorCulture.set", selectedCulture); | |
| await JS.InvokeVoidAsync("setCultureCookie", selectedCulture); | |
| try | |
| { | |
| await JS.InvokeVoidAsync("blazorCulture.set", selectedCulture); | |
| await JS.InvokeVoidAsync("setCultureCookie", selectedCulture); | |
| } | |
| catch (JSException) | |
| { | |
| // If the culture.js script or functions are unavailable, ignore and continue. | |
| } | |
| catch (Exception) | |
| { | |
| // Swallow unexpected errors from JS interop to avoid breaking navigation. | |
| } |
| <select class="form-select form-select-sm" value="@selectedCulture" @onchange="OnChange" style="min-width:120px"> | ||
| <option value="en-US">English</option> | ||
| <option value="zh-CN">中文(简体)</option> | ||
|
|
||
| </select> No newline at end of file |
There was a problem hiding this comment.
The culture switcher select element lacks an accessible label. Screen reader users won't know what this dropdown is for. Add either a visible label, an aria-label attribute, or associate it with a label element using aria-labelledby to describe its purpose (e.g., "Select Language" or "Language Selector").
| <button @onclick="@(e => RuleDelete.InvokeAsync(rule))" class="btn btn-secondary"> | ||
| <button @onclick="@(e => RuleDelete.InvokeAsync(rule))" class="btn btn-secondary" title="@RulesEngineEditor.Resources.SharedResources.Delete" aria-label="@RulesEngineEditor.Resources.SharedResources.Delete"> | ||
| <span class="oi oi-trash"></span> | ||
| <span>@RulesEngineEditor.Resources.SharedResources.Delete</span> |
There was a problem hiding this comment.
The Delete button now contains both an icon and visible text, which may break the UI layout. The header comment at line 22 shows the Delete column header was intentionally commented out, suggesting the design expects icon-only buttons without text labels. Adding visible text (line 62) alongside the icon could cause layout issues in the grid or overflow problems. Consider removing the text span and relying only on the icon with the aria-label and title attributes for accessibility.
| <span>@RulesEngineEditor.Resources.SharedResources.Delete</span> |
| private string LocalizeError(string message) | ||
| { | ||
| if (string.IsNullOrEmpty(message)) return message; | ||
| var m = message; | ||
| m = m.Replace("Validation failed:", RulesEngineEditor.Resources.SharedResources.ValidationFailed); | ||
| m = m.Replace("Severity: Error", RulesEngineEditor.Resources.SharedResources.SeverityError); | ||
| m = m.Replace("Workflow name can not be null or empty", RulesEngineEditor.Resources.SharedResources.WorkflowNameCannotBeEmpty); | ||
| m = m.Replace("Rule Name can not be null", RulesEngineEditor.Resources.SharedResources.RuleNameCannotBeNull); | ||
| m = m.Replace("Atleast one of Rules or WorkflowsToInject must be not empty", RulesEngineEditor.Resources.SharedResources.RulesOrWorkflowsToInjectRequired); | ||
| m = m.Replace("Value cannot be null.", RulesEngineEditor.Resources.SharedResources.ValueCannotBeNull); | ||
| m = m.Replace("(Parameter 'key')", RulesEngineEditor.Resources.SharedResources.ParameterKey); | ||
| m = m.Replace("(Parameter 'value')", RulesEngineEditor.Resources.SharedResources.ParameterValue); | ||
| m = m.Replace("Expression cannot be null or empty when RuleExpressionType is LambdaExpression", RulesEngineEditor.Resources.SharedResources.ExpressionCannotBeNullWhenLambda); | ||
| if (m.Contains("The input does not contain any JSON tokens")) | ||
| m = m.Replace("The input does not contain any JSON tokens", RulesEngineEditor.Resources.SharedResources.JsonNoTokens); | ||
| if (m.Contains("Expected the input to start with a valid JSON token")) | ||
| m = m.Replace("Expected the input to start with a valid JSON token", RulesEngineEditor.Resources.SharedResources.JsonExpectedValidToken); | ||
| if (m.Contains("JSON value could not be converted")) | ||
| m = m.Replace("JSON value could not be converted", RulesEngineEditor.Resources.SharedResources.JsonValueCouldNotBeConverted); | ||
| return m; |
There was a problem hiding this comment.
The LocalizeError method performs multiple sequential string replacements that could interfere with each other if an error message contains text that matches multiple patterns. For example, if a message contains both "Validation failed:" and other error strings, the replacements could produce unexpected results. Consider using a more structured approach such as a dictionary of pattern-to-resource mappings with precedence ordering, or use regex with word boundaries to avoid partial matches.
| catch (Exception ex) | ||
| { | ||
| inputJSONErrors += ex.Message + " "; | ||
| inputJSONErrors += LocalizeError(ex.Message) + " "; |
There was a problem hiding this comment.
String concatenation in loop: use 'StringBuilder'.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@xinshoushangdao Thank you for the pr! |
No description provided.