feat(ValidateForm): reduce render improve performance #7439
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideThis PR improves validation rendering performance and table paging robustness by introducing conditional rendering in ValidateBase, adjusting validation result toggling, and simplifying paging calculations and data materialization, along with updating unit tests to reflect the new validation triggering and markup. Sequence diagram for ValidateBase conditional render on validation togglesequenceDiagram
actor User
participant ValidateForm
participant ValidateBase
participant BlazorRenderer
User ->> ValidateForm: trigger validation (submit form)
ValidateForm ->> ValidateBase: ToggleMessage(results)
ValidateBase ->> ValidateBase: update internal validation state
ValidateBase ->> ValidateBase: _shouldRender = true
ValidateBase ->> BlazorRenderer: StateHasChanged()
BlazorRenderer ->> ValidateBase: ShouldRender()
alt ValidateForm is not null and _shouldRender is true
ValidateBase ->> BlazorRenderer: return true
BlazorRenderer ->> ValidateBase: render component
BlazorRenderer ->> ValidateBase: OnAfterRenderAsync(firstRender false)
ValidateBase ->> ValidateBase: if _shouldRender == false then _shouldRender = null
else ValidateForm is not null and _shouldRender is null or false
ValidateBase ->> BlazorRenderer: return false
BlazorRenderer ->> ValidateBase: skip render
end
Class diagram for updated ValidateBase rendering behaviorclassDiagram
class ValidateBase {
<<component>>
bool? _shouldRender
SetParametersAsync(parameters ParameterView) Task
OnInitialized() void
OnParametersSet() void
ShouldRender() bool
OnAfterRenderAsync(firstRender bool) Task
ToggleMessage(results IReadOnlyCollection~ValidationResult~) Task
}
class ValidateForm {
<<component>>
ToggleMessage(results IReadOnlyCollection~ValidationResult~) Task
}
ValidateForm --> ValidateBase : manages
class ValidationResult {
}
ValidateBase --> ValidationResult : uses
ValidateBase --> ValidateForm : optional reference
Flow diagram for updated table paging recalculation after deletionsflowchart TD
A["DeleteItemsAsync after successful deletion"] --> B["Compute remainingRows = TotalCount - SelectedRows.Count"]
B --> C["Recalculate PageIndex = max(1, min(PageIndex, int(ceil(remainingRows * 1.0 / _pageItems))))"]
C --> D["Filter PageItemsSource where item >= remainingRows and materialize to list"]
D --> E{"items.Count > 0"}
E -- Yes --> F["_pageItems = min(_pageItems, items.Min())"]
E -- No --> G["Keep existing _pageItems"]
F --> H["Continue with table refresh"]
G --> H["Continue with table refresh"]
subgraph DynamicQueryPaging
I["After querying items in QueryDynamicItems"] --> J["TotalCount = items.Count()
PageCount = int(ceil(TotalCount * 1.0 / max(1, _pageItems)))"]
J --> K["Compute remainingRows = TotalCount - SelectedRows.Count"]
K --> L["PageIndex = max(1, min(PageIndex, int(ceil(remainingRows * 1.0 / _pageItems))))"]
L --> M["items = items.Skip((PageIndex - 1) * _pageItems).Take(_pageItems)"]
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new ShouldRender/_shouldRender logic in ValidateBase is a bit hard to follow with the nullable bool tri-state; consider either documenting the intended state transitions or simplifying to a clearer explicit state (e.g., an enum or a non-nullable flag) to make the render throttling behavior easier to reason about.
- In the pagination calculations, you can avoid the ToString/CultureInfo.InvariantCulture/int.Parse round-trip entirely by casting the Math.Ceiling result directly to int (or using Convert.ToInt32), which is simpler and avoids any string-based culture concerns.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new ShouldRender/_shouldRender logic in ValidateBase is a bit hard to follow with the nullable bool tri-state; consider either documenting the intended state transitions or simplifying to a clearer explicit state (e.g., an enum or a non-nullable flag) to make the render throttling behavior easier to reason about.
- In the pagination calculations, you can avoid the ToString/CultureInfo.InvariantCulture/int.Parse round-trip entirely by casting the Math.Ceiling result directly to int (or using Convert.ToInt32), which is simpler and avoids any string-based culture concerns.
## Individual Comments
### Comment 1
<location> `test/UnitTest/Components/ValidateFormTest.cs:546-547` </location>
<code_context>
- await cut.InvokeAsync(() => form.Validate());
- Assert.Contains("form-control valid is-invalid", cut.Markup);
+ var form = cut.Find("form");
+ await cut.InvokeAsync(() => form.Submit());
+ Assert.Contains("form-control invalid is-invalid", cut.Markup);
}
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the assertion to also verify that the "valid" CSS class is no longer present after submission.
The new expectation for `"form-control invalid is-invalid"` is good, but it no longer verifies that the previous `valid` state is cleared. To make the behavior explicit and guard against regressions, add an assertion like:
```csharp
Assert.DoesNotContain("form-control valid", cut.Markup);
```
after submission so the test ensures the control switches from `valid` to `invalid` rather than allowing both states at once.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/Validate/ValidateBase.cs:484` </location>
<code_context>
}
}
+ private bool? _shouldRender = null;
+
/// <summary>
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the nullable tri-state `_shouldRender` and its reset in `OnAfterRenderAsync` with two simple boolean flags that clearly control initial and requested renders.
You can keep the new behavior while removing the nullable tri‑state and the `OnAfterRenderAsync` reset logic by switching to a simple boolean “pending render” flag plus a “first render done” flag.
That keeps all functionality (including the “only rerender when requested inside `ValidateForm`”) and concentrates rendering logic in one place, with no side‑effects in `OnAfterRenderAsync`.
### Suggested refactor
```csharp
// Replace:
private bool? _shouldRender = null;
// With:
private bool _pendingToggleRender;
private bool _hasRendered;
```
```csharp
// Replace the current ShouldRender with:
protected override bool ShouldRender()
{
if (ValidateForm == null)
{
return true;
}
// Always allow the very first render
if (!_hasRendered)
{
_hasRendered = true;
return true;
}
if (_pendingToggleRender)
{
_pendingToggleRender = false;
return true;
}
return false;
}
```
```csharp
// Simplify OnAfterRenderAsync by removing the _shouldRender reset:
protected override async Task OnAfterRenderAsync(bool firstRender)
{
await base.OnAfterRenderAsync(firstRender);
if (!firstRender && IsValid.HasValue)
{
var valid = IsValid.Value;
if (valid)
{
await RemoveValidResult();
}
else
{
await ShowValidResult();
}
}
}
```
```csharp
// In ToggleMessage, reintroduce StateHasChanged and set the simple flag:
public virtual Task ToggleMessage(IReadOnlyCollection<ValidationResult> results)
{
if (FieldIdentifier != null)
{
var messages = results
.Where(item => item.MemberNames.Any(m => m == FieldIdentifier.Value.FieldName))
.ToList();
if (messages.Count > 0)
{
ErrorMessage = messages[0].ErrorMessage;
IsValid = false;
}
else
{
ErrorMessage = null;
IsValid = true;
}
OnValidate(IsValid);
}
_pendingToggleRender = true;
StateHasChanged();
return Task.CompletedTask;
}
```
This:
- Removes the `bool?` tri‑state and the `true → false → null` cycle.
- Keeps `ShouldRender` as a straightforward gate based on two booleans.
- Eliminates side‑effects from `OnAfterRenderAsync`.
- Keeps initial render and “rerender only when `ToggleMessage` requests it” inside `ValidateForm`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7439 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 32894 32911 +17
Branches 4570 4573 +3
=========================================
+ Hits 32894 32911 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces performance improvements to the ValidateForm component by implementing a custom ShouldRender mechanism to reduce unnecessary re-renders. The changes also include culture-aware string formatting in the Table component and corresponding test updates.
- Implements a controlled render mechanism in ValidateBase using a
_shouldRenderstate flag to skip unnecessary renders when inside a ValidateForm - Replaces
StateHasChanged()with state flag manipulation in the validation toggle logic - Updates Table.razor.Toolbar.cs to use
CultureInfo.InvariantCulturefor numeric formatting to ensure consistent behavior across different cultures
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Validate/ValidateBase.cs | Adds ShouldRender override with _shouldRender flag to control rendering, replaces StateHasChanged call with flag manipulation in ToggleMessage, and updates documentation comments to use inheritdoc |
| test/UnitTest/Components/ValidateFormTest.cs | Updates test to use form.Submit() instead of form.Validate(), corrects assertion from "valid" to "invalid" to match expected behavior, and removes trailing whitespace |
| src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs | Adds CultureInfo.InvariantCulture to ToString() calls for culture-independent numeric parsing, replaces Any() with Count > 0 after ToList() materialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This reverts commit 24130a4.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new
_shouldRenderstate machine inValidateBase.ShouldRenderis a bit hard to reason about (nullable + multiple transitions in different methods); consider restructuring it into a clearer explicit state (e.g., enum or clearly documented states) to make future changes less error-prone. - In the table paging logic, the
int.Parse(Math.Ceiling(...).ToString(CultureInfo.InvariantCulture))pattern can be simplified to avoid string allocation/parsing entirely (e.g.,(int)Math.Ceiling(...)), which will both clarify intent and slightly improve performance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `_shouldRender` state machine in `ValidateBase.ShouldRender` is a bit hard to reason about (nullable + multiple transitions in different methods); consider restructuring it into a clearer explicit state (e.g., enum or clearly documented states) to make future changes less error-prone.
- In the table paging logic, the `int.Parse(Math.Ceiling(...).ToString(CultureInfo.InvariantCulture))` pattern can be simplified to avoid string allocation/parsing entirely (e.g., `(int)Math.Ceiling(...)`), which will both clarify intent and slightly improve performance.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Validate/ValidateBase.cs:335-344` </location>
<code_context>
+ /// <returns></returns>
+ protected override bool ShouldRender()
+ {
+ if (ValidateForm == null)
+ {
+ return true;
+ }
+
+ if (_shouldRender is true)
+ {
+ _shouldRender = false;
+ return true;
+ }
+
+ return _shouldRender ?? true;
+ }
+
</code_context>
<issue_to_address>
**issue:** ToggleMessage no longer forces a render when ValidateForm is null, which may change behavior for standalone usage.
Previously `ToggleMessage` always forced a render via `StateHasChanged()`, even without a `ValidateForm` parent. Now `_shouldRender` is ignored when `ValidateForm == null` because `ShouldRender()` just returns `true`, so standalone usage may no longer trigger a render when the internal state changes. If the goal is to optimize only when hosted in `ValidateForm`, consider retaining a `StateHasChanged()` call (or equivalent) when `ValidateForm == null` to preserve the prior behavior for standalone scenarios.
</issue_to_address>
### Comment 2
<location> `test/UnitTest/Components/ValidateFormTest.cs:545-547` </location>
<code_context>
- var form = cut.Instance;
- await cut.InvokeAsync(() => form.Validate());
- Assert.Contains("form-control valid is-invalid", cut.Markup);
+ var form = cut.Find("form");
+ await cut.InvokeAsync(() => form.Submit());
+ Assert.Contains("form-control invalid is-invalid", cut.Markup);
}
</code_context>
<issue_to_address>
**suggestion (testing):** Make the CSS expectations in ValidateFromCode_Ok less brittle and ensure both code-driven and DOM-driven submission paths are covered.
This now exercises only the DOM submission path and relies on the exact concatenated class string. To make the test more robust and clearer:
1. Assert on the presence of the individual classes (e.g., "invalid" and "is-invalid") on the rendered input element rather than on the full class string/order.
2. Add (or parameterize) a companion test that uses the component’s code-driven validation entry point (e.g., `form.Validate()`) so both submission mechanisms are verified to produce the same validation state.
Suggested implementation:
```csharp
});
Assert.Contains("form-control valid", cut.Markup);
// Code-driven validation path
var validateForm = cut.Instance;
await cut.InvokeAsync(() => validateForm.Validate());
var input = cut.Find("input");
var cssClasses = input.GetAttribute("class") ?? string.Empty;
Assert.Contains("form-control", cssClasses);
Assert.Contains("invalid", cssClasses);
Assert.Contains("is-invalid", cssClasses);
// DOM-driven submission path
var form = cut.Find("form");
await cut.InvokeAsync(() => form.Submit());
input = cut.Find("input");
cssClasses = input.GetAttribute("class") ?? string.Empty;
Assert.Contains("form-control", cssClasses);
Assert.Contains("invalid", cssClasses);
Assert.Contains("is-invalid", cssClasses);
}
[Fact]
var msg = cut.FindComponent<MockInput<string>>().Instance.GetErrorMessage();
```
Depending on the full contents of `ValidateFromCode_Ok`, you may want to split this into two tests rather than exercising both paths in one:
1. Keep `ValidateFromCode_Ok` for the code-driven path, calling `cut.Instance.Validate()` and asserting the individual CSS classes as shown.
2. Add a new test, e.g. `ValidateFromDom_Ok`, that performs the DOM `form.Submit()` path and reuses the same class-assertion logic.
If you choose to split the tests, adjust the method bodies accordingly rather than keeping both code paths in a single test. Also verify that the expected final state for this test is indeed the “invalid” state; if `ValidateFromCode_Ok` is supposed to assert a valid state, move the “invalid” assertions to a separate test and add corresponding “valid” class assertions (`valid` instead of `invalid`/`is-invalid`) where appropriate.
</issue_to_address>
### Comment 3
<location> `src/BootstrapBlazor/Components/Validate/ValidateBase.cs:484` </location>
<code_context>
}
}
+ private bool? _shouldRender = null;
+
/// <summary>
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the nullable tri-state `_shouldRender` logic with two explicit render flags managed solely in `ShouldRender` to make the render flow easier to follow.
You can keep the behavior while removing the tri‑state state machine and the cross‑lifecycle mutation, which is the main source of cognitive load.
Instead of `bool? _shouldRender` with values `{ null, true, false }` spread across `ShouldRender`, `OnAfterRenderAsync`, and `ToggleMessage`, you can represent the same behavior with two explicit flags handled entirely in `ShouldRender`:
```csharp
// replaces: private bool? _shouldRender = null;
private bool _pendingValidationRender;
private bool _suppressNextRender;
```
```csharp
public virtual Task ToggleMessage(IReadOnlyCollection<ValidationResult> results)
{
if (FieldIdentifier != null)
{
var messages = results.Where(item => item.MemberNames.Any(m => m == FieldIdentifier.Value.FieldName)).ToList();
if (messages.Count > 0)
{
ErrorMessage = messages[0].ErrorMessage;
IsValid = false;
}
else
{
ErrorMessage = null;
IsValid = true;
}
OnValidate(IsValid);
}
// Must refresh UI once to ensure state is correct
_pendingValidationRender = true;
return Task.CompletedTask;
}
```
```csharp
protected override bool ShouldRender()
{
if (ValidateForm == null)
{
return true;
}
// First render after ToggleMessage
if (_pendingValidationRender)
{
_pendingValidationRender = false;
_suppressNextRender = true;
return true;
}
// One suppressed render (previously _shouldRender == false)
if (_suppressNextRender)
{
_suppressNextRender = false;
return false;
}
// Default behavior (previously _shouldRender == null)
return true;
}
```
```csharp
protected override async Task OnAfterRenderAsync(bool firstRender)
{
await base.OnAfterRenderAsync(firstRender);
if (!firstRender && IsValid.HasValue)
{
var valid = IsValid.Value;
if (valid)
{
await RemoveValidResult();
}
else
{
await ShowValidResult();
}
}
// No need to touch render flags here anymore
}
```
This keeps:
- The “one extra render and then suppress the next” behavior tied to `ToggleMessage`.
- Default `true` rendering when `ValidateForm == null`.
- No change to validation / JS logic.
But it removes:
- The nullable `bool?` tri‑state semantics.
- The need to mentally trace `_shouldRender` transitions across `ToggleMessage`, `ShouldRender`, and `OnAfterRenderAsync`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Validate/ValidateBase.cs:333-342` </location>
<code_context>
+ /// <inheritdoc/>
+ /// </summary>
+ /// <returns></returns>
+ protected override bool ShouldRender()
+ {
+ if (ValidateForm == null)
+ {
+ return true;
+ }
+
+ if (_shouldRender is true)
+ {
+ _shouldRender = false;
+ return true;
+ }
+
+ return _shouldRender ?? true;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Setting `_shouldRender` alone does not trigger a re-render; removing `StateHasChanged()` in `ToggleMessage` likely breaks the “must refresh once UI” guarantee.
The original `ToggleMessage` forced a re-render via `StateHasChanged()` to satisfy the “must refresh once UI” requirement. Now `_shouldRender` is set but no render is requested, and `ShouldRender` only runs when a render is already in progress. Unless another code path always triggers a render after `ToggleMessage`, the UI may not update on validation changes. Please either reintroduce a `StateHasChanged()` call (optionally conditioned on `_shouldRender`) or add an equivalent mechanism to request a render when `_shouldRender` is toggled.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs:1090` </location>
<code_context>
- PageIndex = Math.Max(1, Math.Min(PageIndex, int.Parse(Math.Ceiling((TotalCount - SelectedRows.Count) * 1d / _pageItems).ToString())));
- var items = PageItemsSource.Where(item => item >= (TotalCount - SelectedRows.Count));
- if (items.Any())
+ PageIndex = Math.Max(1, Math.Min(PageIndex, int.Parse(Math.Ceiling((TotalCount - SelectedRows.Count) * 1d / _pageItems).ToString(CultureInfo.InvariantCulture))));
+ var items = PageItemsSource.Where(item => item >= (TotalCount - SelectedRows.Count)).ToList();
+ if (items.Count > 0)
</code_context>
<issue_to_address>
**suggestion (performance):** The `PageIndex` calculation can be simplified to avoid string conversions, like in the later change.
This still uses `int.Parse(Math.Ceiling(...).ToString(CultureInfo.InvariantCulture))`, whereas below you switched to `(int)Math.Ceiling(...)`. For consistency and to avoid unnecessary string allocations and parsing, please update this line to the same pattern: `(int)Math.Ceiling((TotalCount - SelectedRows.Count) * 1.0 / _pageItems)`.
```suggestion
PageIndex = Math.Max(1, Math.Min(PageIndex, (int)Math.Ceiling((TotalCount - SelectedRows.Count) * 1.0 / _pageItems)));
```
</issue_to_address>
### Comment 3
<location> `test/UnitTest/Components/ValidateFormTest.cs:545-547` </location>
<code_context>
- var form = cut.Instance;
- await cut.InvokeAsync(() => form.Validate());
- Assert.Contains("form-control valid is-invalid", cut.Markup);
+ var form = cut.Find("form");
+ await cut.InvokeAsync(() => form.Submit());
+ Assert.Contains("form-control invalid is-invalid", cut.Markup);
}
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the updated validation test assertions and make them more robust against async rendering.
The assertion currently runs immediately after `form.Submit()`, which may race with async rendering. Consider wrapping the assertion in `cut.WaitForAssertion(() => Assert.Contains("form-control invalid is-invalid", cut.Markup));` (or equivalent) so the test waits for the DOM update. You could also assert that `"form-control valid"` is absent (e.g. `Assert.DoesNotContain("form-control valid", cut.Markup);`) to verify the state actually transitioned from valid to invalid and reduce flakiness.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
_shouldRender/ShouldRenderlogic is fairly subtle; consider adding a brief comment explaining the tri-state behavior (null/true/false) and when it is expected to be toggled (e.g., viaToggleMessageandOnAfterRenderAsync) to make future maintenance safer. - The PageIndex recalculation logic in
DeleteItemsAsyncandQueryDynamicItemsis now duplicated and slightly complex; extracting this into a shared helper (e.g.,RecalculatePageIndexAfterDeletion) would reduce repetition and make the paging behavior easier to reason about.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `_shouldRender`/`ShouldRender` logic is fairly subtle; consider adding a brief comment explaining the tri-state behavior (null/true/false) and when it is expected to be toggled (e.g., via `ToggleMessage` and `OnAfterRenderAsync`) to make future maintenance safer.
- The PageIndex recalculation logic in `DeleteItemsAsync` and `QueryDynamicItems` is now duplicated and slightly complex; extracting this into a shared helper (e.g., `RecalculatePageIndexAfterDeletion`) would reduce repetition and make the paging behavior easier to reason about.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Validate/ValidateBase.cs:333` </location>
<code_context>
+ /// <inheritdoc/>
+ /// </summary>
+ /// <returns></returns>
+ protected override bool ShouldRender()
+ {
+ if (ValidateForm == null)
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the nullable tri-state `_shouldRender` and its reset in `OnAfterRenderAsync` with a single boolean flag that forces exactly one extra render when needed.
You can drop the tri‑state and the `OnAfterRenderAsync` mutation entirely and keep the “force next render once” behavior with a single boolean flag. The current `false → null` transition via `OnAfterRenderAsync` is effectively an implementation detail that never affects `ShouldRender` decisions (since `_shouldRender == false` is only seen in the same render cycle where it was set).
A simpler version:
```csharp
private bool _forceNextRender;
protected override bool ShouldRender()
{
if (ValidateForm == null)
{
return true;
}
if (_forceNextRender)
{
_forceNextRender = false;
return true;
}
// Fall back to the base behavior (usually "always render")
return base.ShouldRender();
}
```
Then `ToggleMessage` becomes:
```csharp
public virtual Task ToggleMessage(IReadOnlyCollection<ValidationResult> results)
{
if (FieldIdentifier != null)
{
var messages = results.Where(item => item.MemberNames.Any(m => m == FieldIdentifier.Value.FieldName)).ToList();
if (messages.Count > 0)
{
ErrorMessage = messages.Select(item => item.ErrorMessage).FirstOrDefault();
IsValid = false;
}
else
{
ErrorMessage = null;
IsValid = true;
}
OnValidate(IsValid);
}
// Force a single refresh of the UI
_forceNextRender = true;
StateHasChanged();
return Task.CompletedTask;
}
```
And you can remove the extra state logic from `OnAfterRenderAsync`:
```csharp
protected override async Task OnAfterRenderAsync(bool firstRender)
{
await base.OnAfterRenderAsync(firstRender);
if (!firstRender && IsValid.HasValue)
{
var valid = IsValid.Value;
if (valid)
{
await RemoveValidResult();
}
else
{
await ShowValidResult();
}
}
}
```
This keeps all existing functionality (including the “one forced render after `ToggleMessage`”) while:
- Eliminating the nullable tri‑state,
- Localizing state transitions to `ToggleMessage` and `ShouldRender`,
- Removing the non‑obvious `false → null` reset from `OnAfterRenderAsync`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This was referenced Dec 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #7438
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Optimize validation rendering behavior and adjust table pagination calculations to improve performance and correctness.
Bug Fixes:
Enhancements:
Tests: