Skip to content

Conversation

@DrastamatSargsyan
Copy link
Contributor

Enhances the control of tab header mouse clicks by adding the onmousedown and oncontextmenu events.
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:

  • The PR is submitted to the correct branch (dev).
  • My code follows the style of this project.
  • I've added relevant tests or confirmed existing ones.

@mudbot mudbot bot changed the title MudTabs: Add onmousedown and oncontextmenu events (#11190) MudTabs: Add onmousedown and oncontextmenu events Oct 17, 2025
@mudbot mudbot bot added the enhancement Request for adding a new feature or enhancing existing functionality (not fixing a defect) label Oct 17, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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.

@versile2
Copy link
Contributor

How does this affect Disabled Tabs? Are there other edge cases we should look at?

@DrastamatSargsyan
Copy link
Contributor Author

How does this affect Disabled Tabs?

As I can see, none of the events is being triggered if the tab is disabled, and I didn't quite catch why.

@versile2
Copy link
Contributor

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?

Copy link
Contributor

Copilot AI left a 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.

@DrastamatSargsyan
Copy link
Contributor Author

Do we need both or can we change that onclick to fire Activate with the mousedown?

My first thought was that, but I didn't want to make a change like that, as the @onclick carries the ActivatePanels logic on it. Instead, the new handlers will provide additional control over the tab, as we normally use the @onmousedown right away on other components, which unfortunately is not working with the MudTabPanel (https://try.mudblazor.com/snippet/cEQJFOcawqEXwDgO)

Copy link
Contributor

@versile2 versile2 left a 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.

@danielchalmers
Copy link
Member

Should we support pointer down instead of the mouse specifically? For touch screens etc

@mudbot mudbot bot added the needs: changes A maintainer has asked for further modifications to be made before this pull request can be approved label Oct 22, 2025
@ScarletKuro
Copy link
Member

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)

@danielchalmers
Copy link
Member

danielchalmers commented Oct 22, 2025

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?

@DrastamatSargsyan
Copy link
Contributor Author

so any event/attribute can be used without needing new properties?

But it already does work with the MudButton, right? What's the issue with the MudTabPanel?

A little example showing the difference - https://try.mudblazor.com/snippet/cEQJFOcawqEXwDgO

@versile2
Copy link
Contributor

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)

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

Copy link
Contributor

Copilot AI left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Request for adding a new feature or enhancing existing functionality (not fixing a defect) needs: changes A maintainer has asked for further modifications to be made before this pull request can be approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants