MudTabs: Fix scroll buttons inside disabled form#12885
MudTabs: Fix scroll buttons inside disabled form#12885danielchalmers merged 4 commits intoMudBlazor:devfrom
Conversation
- Create ScrollButtons viewer test and unit test - Add fixed CascadingValue ParentDisabled with false value aroud the scroll buttons
There was a problem hiding this comment.
Pull request overview
Fixes a MudTabs regression where tab scroll buttons become disabled when MudTabs is wrapped in a disabled MudForm (via the ParentDisabled cascading value), ensuring scroll navigation remains usable.
Changes:
- Wrap MudTabs scroll buttons with a fixed
CascadingValue Name="ParentDisabled"set tofalseto prevent form-level disabled state from disabling them. - Add a unit test covering the regression scenario.
- Add a new UnitTests.Viewer repro component to demonstrate the issue.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/MudBlazor/Components/Tabs/MudTabs.razor | Overrides ParentDisabled cascade for scroll buttons so they remain interactive inside disabled forms. |
| src/MudBlazor.UnitTests/Components/TabsTests.cs | Adds a regression test intended to verify scroll buttons remain enabled under a disabled parent form. |
| src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/TabScrollButtonsEnabledInsideFormTest.razor | Adds a viewer repro for scroll-button behavior inside a disabled MudForm. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| scrollButtons.First().Instance.Disabled.Should().BeFalse("scroll button should be enabled initially"); | ||
| scrollButtons.Last().Instance.Disabled.Should().BeFalse("scroll button should be enabled initially"); | ||
|
|
||
| var toggleFormDisabledButton = comp.Find("button.mud-button-root:not(.mud-icon-button)"); | ||
| await toggleFormDisabledButton.ClickAsync(); | ||
|
|
||
| scrollButtons = comp.FindComponents<MudIconButton>(); | ||
| scrollButtons.First().Instance.Disabled.Should().BeFalse("scroll button should remain enabled even when form is disabled"); | ||
| scrollButtons.Last().Instance.Disabled.Should().BeFalse("scroll button should remain enabled even when form is disabled"); | ||
|
|
||
| await toggleFormDisabledButton.ClickAsync(); | ||
|
|
||
| scrollButtons = comp.FindComponents<MudIconButton>(); | ||
| scrollButtons.First().Instance.Disabled.Should().BeFalse("scroll button should be enabled when form is re-enabled"); | ||
| scrollButtons.Last().Instance.Disabled.Should().BeFalse("scroll button should be enabled when form is re-enabled"); |
There was a problem hiding this comment.
The new test assumes both scroll buttons are enabled (Disabled == false) initially and after toggling the form. In MudTabs, the previous button is legitimately disabled when the scroll position is at the start (see SetScrollabilityStates: _prevButtonDisabled = Math.Abs(_scrollPosition) < 0.01), so this assertion can fail even when the ParentDisabled regression is fixed. Instead, capture the buttons' Disabled states before disabling the form and assert the states do not change when the parent form toggles, or move the scroll position (e.g., click Next once) so the button-under-test is enabled due to scroll state and then verify it remains enabled under a disabled form.
| scrollButtons.First().Instance.Disabled.Should().BeFalse("scroll button should be enabled initially"); | |
| scrollButtons.Last().Instance.Disabled.Should().BeFalse("scroll button should be enabled initially"); | |
| var toggleFormDisabledButton = comp.Find("button.mud-button-root:not(.mud-icon-button)"); | |
| await toggleFormDisabledButton.ClickAsync(); | |
| scrollButtons = comp.FindComponents<MudIconButton>(); | |
| scrollButtons.First().Instance.Disabled.Should().BeFalse("scroll button should remain enabled even when form is disabled"); | |
| scrollButtons.Last().Instance.Disabled.Should().BeFalse("scroll button should remain enabled even when form is disabled"); | |
| await toggleFormDisabledButton.ClickAsync(); | |
| scrollButtons = comp.FindComponents<MudIconButton>(); | |
| scrollButtons.First().Instance.Disabled.Should().BeFalse("scroll button should be enabled when form is re-enabled"); | |
| scrollButtons.Last().Instance.Disabled.Should().BeFalse("scroll button should be enabled when form is re-enabled"); | |
| var initialPrevDisabled = scrollButtons.First().Instance.Disabled; | |
| var initialNextDisabled = scrollButtons.Last().Instance.Disabled; | |
| var toggleFormDisabledButton = comp.Find("button.mud-button-root:not(.mud-icon-button)"); | |
| await toggleFormDisabledButton.ClickAsync(); | |
| scrollButtons = comp.FindComponents<MudIconButton>(); | |
| scrollButtons.First().Instance.Disabled.Should().Be(initialPrevDisabled, "scroll button disabled state should not change when form is disabled"); | |
| scrollButtons.Last().Instance.Disabled.Should().Be(initialNextDisabled, "scroll button disabled state should not change when form is disabled"); | |
| await toggleFormDisabledButton.ClickAsync(); | |
| scrollButtons = comp.FindComponents<MudIconButton>(); | |
| scrollButtons.First().Instance.Disabled.Should().Be(initialPrevDisabled, "scroll button disabled state should not change when form is re-enabled"); | |
| scrollButtons.Last().Instance.Disabled.Should().Be(initialNextDisabled, "scroll button disabled state should not change when form is re-enabled"); |
There was a problem hiding this comment.
I don't think the suggestion is the best, maybe the point can make sense, but I think with de MudContainer with a extra small max width, and if I add more tab panels, I can guarantee the scroll buttons are gone be enabled by default
src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/TabScrollButtonsEnabledInsideFormTest.razor
Show resolved
Hide resolved
src/MudBlazor.UnitTests.Viewer/TestComponents/Tabs/TabScrollButtonsEnabledInsideFormTest.razor
Outdated
Show resolved
Hide resolved
…ttonsEnabledInsideFormTest.razor Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
changed to waitforassertionasync pattern
There's a bug that if a Disabled Form wrap MudTabs the scroll buttons are disabled because of the CascadingValue ParentDisabled, so I added around only the scroll buttons an fixed CascadingValue ParentDisabled with value false to prevent this bug to happens, the issue
fixes #12366
video.mp4
Checklist:
I've read the contribution guidelines
My code follows the style of this project
I've added or updated relevant unit tests
Verified tabs are considered navigation and general consensus is not to be disabled per MUI standards. There was not a hard and fast standard for it but that seems to be the generally accepted practice.