Codecov Report

Merging #4801 (52a2105) into dev (6dbc08c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              dev    #4801   +/-   ##
=======================================
  Coverage   91.41%   91.42%           
=======================================
  Files         365      365           
  Lines       12542    12553   +11     
=======================================
+ Hits        11465    11476   +11     
  Misses       1077     1077           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6dbc08c...52a2105. Read the comment docs.

@henon Is there anything you would like me to change here?

I can see how this can be useful.

The only thing I worry is that this event will cause a re-render of the whole form whenever the user changes something. It might cause unwanted effects or even slow down big forms. In general it can be dangerous for elements to cause render updates on their ancestors.

In this light, can you make sure raising this event doesn't cause form re-renders when a user doesn't add a handler for this event? It might be this way already but we better check. For instance, in a testcase you can access the RenderCount property of a TestComponent.

One potential issue: MudTextFields trigger the event twice, once with a MudTextField type and once with a MudInput type.

We have a solution for that. You can check the Standalone cascading parameter of Textfield / Input. If Standalone is false you should not fire the event. You'll notice that the Standalone cascading parameter is already used to prevent internal Inputs to do validation.

Lastly, this seems like a niche change and I have not written proper documentation for it. However, I can do so if desired.

I think a good doc string on the event is enough.

In this light, can you make sure raising this event doesn't cause form re-renders when a user doesn't add a handler for this event? It might be this way already but we better check. For instance, in a testcase you can access the RenderCount property of a TestComponent.

I have confirmed no renders occur when no handler is assigned to FieldChanged.

I looked into optimizing the renders with ShouldRender as SuppressRenderingOnValidation does but haven't finalized a solution for that. Is this something that should be included, or do we not care because no renders will occur anyway if FieldChanged is unused?

We have a solution for that. You can check the Standalone cascading parameter of Textfield / Input. If Standalone is false you should not fire the event. You'll notice that the Standalone cascading parameter is already used to prevent internal Inputs to do validation.

Perfect, I have implemented this change and the double event trigger is resolved.

I looked into optimizing the renders with ShouldRender as SuppressRenderingOnValidation does but haven't finalized a solution for that. Is this something that should be included, or do we not care because no renders will occur anyway if FieldChanged is unused?

Yeah SuppressRenderingOnValidation was added exactly for the same reason because in some cases the update of the IsValid property in Form caused unwanted re-renders while the user was still typing. But I'd say we can let it be for now as this event won't be handled by most users.