-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
MudTabs: Add onmousedown and oncontextmenu events #11966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
MudTabs: Add onmousedown and oncontextmenu events #11966
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully enhances the MudTabs component by adding onmousedown and oncontextmenu events, which is a great addition for more advanced user interactions. The implementation is solid and is well-supported by a new test component and corresponding unit tests. My review includes a couple of suggestions to align the code with the repository's style guide, focusing on improving consistency in event binding and adhering to naming conventions for asynchronous methods.
src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/ClosableTabsWithHeaderTest.razor
Outdated
Show resolved
Hide resolved
|
How does this affect Disabled Tabs? Are there other edge cases we should look at? |
As I can see, none of the events is being triggered if the tab is disabled, and I didn't quite catch why. |
|
As to the disabled portion it does have mud-disabled applied to that div you are modifying with the handlers which sets cursor to default, pointer-events to none and color. So the TabPanel has both the onmousedown and onclick handlers, Which is firing first? Do we need both or can we change that onclick to fire Activate with the mousedown? There's some subtle interaction that is likely happening here, it resolves okay in your test and doesn't cause a problem though I worry about it. Your thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds onmousedown and oncontextmenu support to MudTabs tab headers so consumers can handle middle-click tab closing and custom context menus.
- Exposes OnMouseDown and OnContextMenu EventCallback parameters on MudTabPanel
- Wires these events in MudTabs.RenderTab, preventing the browser context menu when a handler is provided
- Adds unit tests and a test component demonstrating middle-click close and context menu actions
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/MudBlazor/Components/Tabs/MudTabs.razor | Wires @onmousedown and @oncontextmenu on the tab header div, with preventDefault when a context menu handler is present |
| src/MudBlazor/Components/Tabs/MudTabPanel.razor.cs | Adds OnMouseDown and OnContextMenu parameters with summaries and categories |
| src/MudBlazor.UnitTests/Components/TabsTests.cs | Adds tests to validate middle-click close and context menu actions on tabs |
| src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/ClosableTabsWithHeaderTest.razor | Test component demonstrating tab closing via middle-click and via context menu |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/ClosableTabsWithHeaderTest.razor
Outdated
Show resolved
Hide resolved
My first thought was that, but I didn't want to make a change like that, as the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified visually no changes if not used and no odd effects when used. Code is sound and fairly simple. Good work.
|
Should we support pointer down instead of the mouse specifically? For touch screens etc |
Isn't the problem that it doesn't support middle click? Which sometimes ppl request #11835 (comment) |
|
Would it work if we supported arbitrary attributes through the existing UserAttributes dict: <MudTabs @bind-ActivePanelIndex="Index" Border="true" Outlined="true" PanelClass="px-4 py-6" ApplyEffectsToContainer="true">
<ChildContent>
@foreach (var item in _tabs)
{
- <MudTabPanel Text="@item.Name" Tag="@item.Id" ShowCloseIcon="true" OnMouseDown="@(e => OnMouseDown(e, item))" OnContextMenu="@(e => OpenMenuContentAsync(e, item))">
+ <MudTabPanel Text="@item.Name" Tag="@item.Id" ShowCloseIcon="true" @onmousedown="@(e => OnMouseDown(e, item))" @oncontextmenu="@(e => OpenMenuContentAsync(e, item))">
@item.Content
</MudTabPanel>
}
</ChildContent>
<TabPanelHeader>
@if (context.ShowCloseIcon)
{
<MudIconButton Class="pa-0 mr-3" Icon="@Icons.Material.Filled.Close" OnClick="(_) => RemoveTab(context)" />
}
</TabPanelHeader>
</MudTabs>
// https://github.com/DrastamatSargsyan/MudBlazor/blob/dbbf743eda5ee00df9110408158720bcf110654f/src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/ClosableTabsWithHeaderTest.razor
}- RenderFragment RenderTab(MudTabPanel panel) => @<div @ref="panel.PanelRef" ...
+ RenderFragment RenderTab(MudTabPanel panel) => @<div @ref="panel.PanelRef" @attributes="panel.UserAttributes" ...so any event/attribute can be used without needing new properties? |
But it already does work with the A little example showing the difference - https://try.mudblazor.com/snippet/cEQJFOcawqEXwDgO |
It should likely be changed to mousedown to match the select/autocomplete rather than onclick imo. Which should give access to middle button and touch. As long as we make sure no side effects doable. V |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/ClosableTabsWithHeaderTest.razor
Outdated
Show resolved
Hide resolved
…DrastamatSargsyan/MudBlazor into feature/tabs-mouse-click-events
Enhances the control of tab header mouse clicks by adding the
onmousedownandoncontextmenuevents.The test example shows the cases of closing the tabs with the mouse middle click and opening a context menu on tabs.
Possibly solves the #11190 second part.
Checklist:
dev).