search

MudBlazor / MudBlazor

Blazor Component Library based on Material design with an emphasis on ease of use. Mainly written in C# with Javascript kept to a bare minimum it empowers .NET developers to easily debug it if needed.
http://mudblazor.com
MIT License
7.87k stars 1.25k forks source link

Button in MudMenuItem closes the menu even though AutoClose is false #8287

Open V1ck3s opened 6 months ago

V1ck3s commented 6 months ago

Bug type

Component

Component name

MudMenuItem

What happened?

When we use a button (of any type) in a MudMenuItem, when we click on it, it does not respect the AutoClose property. On this GIF, all MudMenuItem has AutoClose at false. firefox_2h3Mxyeoac

Expected behavior

The menu should not close.

Reproduction link

https://try.mudblazor.com/snippet/cOcIknYpniQimiZP

Reproduction steps

  1. Create a menu with MudMenuItems and add buttons in it.
  2. Adding AutoClose="false" to MudMenuItems.
  3. Test, click on item work but when we click on buttons, it close the menu. ...

Relevant log output

No response

Version (bug)

6.16.0

Version (working)

No response

What browsers are you seeing the problem on?

Firefox, Chrome, Microsoft Edge

On what operating system are you experiencing the issue?

Windows

Pull Request

Code of Conduct

ScarletKuro commented 6 months ago

@henon I haven't debugged it, but in my theory the problem is in the IActivatable architecture. The MudBaseButton is triggering this: https://github.com/MudBlazor/MudBlazor/blob/5b3757a4c881e4b69b4f8eead488e22c1280d905/src/MudBlazor/Base/MudBaseButton.cs#L135 and then it propagates to MudMenu: https://github.com/MudBlazor/MudBlazor/blob/5b3757a4c881e4b69b4f8eead488e22c1280d905/src/MudBlazor/Components/Menu/MudMenu.razor.cs#L347-L350

But the problem is that the MudMenu doesn't know anything about the AutoClose setting in MudMenuItem that triggered it (technically speaking, it's not even MudMenuItem that triggered it but the Button that is under it) and honestly I don't know how we can we make this info flow. I only have in mind a workaround for MudBaseButton with additional property that won't call the IActivatable.Activate but doesn't sound optimal.

danielchalmers commented 6 months ago

I think I had the same problem in #7213.

8331 has got to be related. Maybe MudButton needs preventDefault as well 🤔

ScarletKuro commented 6 months ago

I think I had the same problem in #7213.

It could be, because the next and previous buttons are mud buttons which is invoking the Activateable, but when you click in the middle (year) then it doesn't close because it's just a native button.

8331 is related but doesn't fix it the way it is. Maybe MudButton needs preventDefault as well 🤔

I don't think that the core problem is same, but that's just my theory.

danielchalmers commented 6 months ago
henon commented 6 months ago

@ScarletKuro You were right! It is indeed the IActivatable framework causing this. There is a workaround. You can create a containter component that implements IActivatable and put the buttons inside of it. Here is the fixed menu: https://try.mudblazor.com/snippet/wYQSanFmRxwqjzLD

henon commented 6 months ago

As long as we don't have a good strategy to fix this I'd say we won't fix.

ScarletKuro commented 6 months ago

My only idea would be to add some ActivatableContext that flows with the CascadingValue, MudMenuItem is modifying it with the AutoClose settings, and mudbutton just forwards the new ActivatableContext. But that would require the interface to change.

danielchalmers commented 5 months ago

My only idea would be to add some ActivatableContext that flows with the CascadingValue, MudMenuItem is modifying it with the AutoClose settings, and mudbutton just forwards the new ActivatableContext. But that would require the interface to change.

@ScarletKuro Can we make that interface change in v7?

henon commented 5 months ago

I don't think that is something we should work on in v7. It is a fringe problem and has a good workaround besides.

danielchalmers commented 5 months ago

@henon @ScarletKuro Just so I understand, why do we have to toggle the menu when Activate is called?

Without ToggleMenu:

https://github.com/MudBlazor/MudBlazor/assets/7112040/88cdd07f-2990-40af-a6b5-780d8ed30ead

henon commented 5 months ago

@danielchalmers I don't understand what you are suggesting. You suggest to remove a line of code from MudMenu that was put there for a reason?

danielchalmers commented 5 months ago

@danielchalmers I don't understand what you are suggesting. You suggest to remove a line of code from MudMenu that was put there for a reason?

@henon No, I'm asking what that reason is.

henon commented 5 months ago

Oh, ok. The IActivatable interface was invented to allow custom action content with buttons. We'd define render fragments the users could fill with their own content and be sure the button would activate the componet. This was conceived especially for things like Close buttons. In the case of Menu, it implements this interface so that you can add a Close-button to it when it doesn't have its own special close button, which would be a nightmare to support out of the box with placing, styling and all. I hope my somewhat incoherent explanation makes sense.