The breaking change is in my opinion minor as it concerns MudElement with the ClickPropagation property not having internal logic.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.20%. Comparing base (28bc599) to head (827b7ae).
Report is 147 commits behind head on dev.

@@            Coverage Diff             @@
##              dev    #8649      +/-   ##
==========================================
+ Coverage   89.82%   90.20%   +0.37%     
==========================================
  Files         412      423      +11     
  Lines       11878    12274     +396     
  Branches     2364     2407      +43     
==========================================
+ Hits        10670    11072     +402     
+ Misses        681      668      -13     
- Partials      527      534       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Another thing that I would like to discuss here.

What is the intended behaviour when both Href and OnClick handlers are present. I think the behaviour should be consistent across all components.

From my understanding, if Href is set the OnClick should not be called. But this decision imposes some troubles. It is for example not easily possible to add an MenuClose handler to inner MudListItem.
It even gets more complex, if we add an Target attribute (e.g. _blank) and or a ForceLoad parameter. What is the expected behaviour for a disabled button/item with an href. Should the Href then also be disabled, by forcing a render as button tag.

For my expectation it would be best to always call the OnClick handler. That might close a menu and Browser client can do a navigation (even to a new tab) -- then the close menu is desired.

I think it is quite important to have a discussion on this intended behaviour, to get a consistent design!

My Pull-Request tried to solve the problem of #1717, but it seems there are more thoughts required!

Sorry, I think my PR #8634 added conflicts to your PR, since I did method renaming, so was the tests affected too.

I've merged with dev and kept the meaning of MudElements ClickPropagation Parameter.

@danielchalmers I followed your recommendations and left the ClickPropagation parameter unchanged. The PreventDefault parameter was added in the same way. Even if the name imposes an inversion, I would stick with that name, as it matches the output attribute.

What is the intended behaviour when both Href and OnClick handlers are present. I think the behaviour should be consistent
From my understanding, if Href is set the OnClick should not be called. But this decision imposes some troubles. It is for example not easily possible to add an MenuClose handler to inner MudListItem. It even gets more complex, if we add an Target attribute (e.g. _blank) and or a ForceLoad parameter. What is the expected behaviour for a disabled button/item with an href. Should the Href then also be disabled, by forcing a render as button tag.

For my expectation it would be best to always call the OnClick handler. That might close a menu and Browser client can do a navigation (even to a new tab) -- then the close menu is desired.

I think it is quite important to have a discussion on this intended behaviour, to get a consistent design!

I agree that calling the OnClick handler regardless of the Href value (except if Disabled is true) makes sense. If I attach a click handler I expect it to fire. If I want to just navigate I don't attach the handler, right?

I agree that calling the OnClick handler regardless of the Href value (except if Disabled is true) makes sense. If I attach a click handler I expect it to fire. If I want to just navigate I don't attach the handler, right?

@henon Isn't it a problem that several other controls do it the other way?

Yeah, that is a problem. I don't understand why it is like that. It is always a bad idea to try to "save the user from its own supposed stupidity". Most of the times you just hinder advanced use-cases for nothing.

@belucha Please resolve conflicts. What's the status? We should get this done for v7

@henon I've just resolved all conflicts.
@danielchalmers I also considered your suggestions, except on regarding the rename of MudElement.ClickPropagation. I left that unchanged, since this was the original API and the the difference between OnClickPropagation and ClickPropagation is quite little.

Thanks!

@henon Thanks to you and the others for the great work on this lib!

Added to v7.0.0 Migration Guide #8447

@henon
I'm so sorry, I found an issue in the code just now, that was not my intention and I somehow missed it.
in MudBaseButton.cs there is a property:

        public bool ApplyClickPropagation => HtmlTag != "button" || ClickPropagation;

this should be private and matching to your convention:

        private bool GetClickPropagation() => HtmlTag != "button" || ClickPropagation;

Do you wan't me to open a new pull request, or do you change that yourself?

Sorry for this mistake.

Don't worry, I am fixing it myself.

After going through this PR again, I noticed that it was requested to add XML documentation for this property: #8649 (comment).

However, it's missing:

Additionally, there is an extra new line and a missing new line after the property.

@belucha, can you please make a PR to fix it?

Sure. I'm sorry if I missed that. I will do so in about two weeks.

Sure. I'm sorry if I missed that. I will do so in about two weeks.

Fixed already #9387
But if something is missing, feel free to PR.