MudOverlay: Fix scrollable background with visible dialog#12121
MudOverlay: Fix scrollable background with visible dialog#12121ScarletKuro merged 2 commits intoMudBlazor:devfrom
Conversation
|
|
||
| // Calling HandleLockScrollChange when the overlay isn't intially set to | ||
| // visible will incorrectly decrement the lock count | ||
| if (Visible) |
There was a problem hiding this comment.
Should be _visibleState.Value, we don't read state properties directly, please verify it works.
There was a problem hiding this comment.
Literally a couple of lines down the code is already doing
if (Visible && !Modal && AutoClose)
This seems somewhat conflicting?
There was a problem hiding this comment.
Seems this was my oversight from when I initially began developing this #8258
This should be changed too.
There was a problem hiding this comment.
Pushed a change that updates property access for the whole class.
Fixes scrollable background with visible dialog & Fix visible property access. Fixes #12078
|
@versile2 mind to double check this later? |
The fix looks good and works. I do believe it should have a unit test attached that verifies lock scroll. I know it's a complex test but if you added the trymud attached to the issue to the viewer project your test would look like the below. I've verified this fails without your changes and passes with your changes. [Test]
public void Overlay_StartClosedTest()
{
var jsRuntimeMock = new Mock<IJSRuntime>(MockBehavior.Loose);
Context.Services.AddSingleton(typeof(IJSRuntime), jsRuntimeMock.Object);
// verifies lockScroll was called once and 2 arguments were supplied
jsRuntimeMock
.Setup(x => x.InvokeAsync<IJSVoidResult>("mudScrollManager.lockScroll", It.Is<object[]>(y => y.Length == 2)))
.ReturnsAsync(Mock.Of<IJSVoidResult>())
.Verifiable();
// Expect unlockScroll to NOT be called
jsRuntimeMock
.Setup(x => x.InvokeAsync<IJSVoidResult>(
"mudScrollManager.unlockScroll",
It.IsAny<object[]>()))
.Throws(new Exception("unlockScroll should not be called!"));
var dialog = Context.RenderComponent<MudDialogProvider>();
var comp = Context.RenderComponent<OverlayScrollLockedTest>();
// click the button opening dialog
var button = comp.Find("button");
button.Click();
// verify dialog is open
comp.WaitForAssertion(() => dialog.FindComponent<MudOverlay>().Should().NotBeNull());
// verify lockScroll was called
jsRuntimeMock.Verify(
x => x.InvokeAsync<IJSVoidResult>(
"mudScrollManager.lockScroll",
It.IsAny<object[]>()),
Times.Once);
// verify unlockScroll was NOT called
jsRuntimeMock.Verify(
x => x.InvokeAsync<IJSVoidResult>(
"mudScrollManager.unlockScroll",
It.IsAny<object[]>()),
Times.Never);
} |
|
Added the provided test. |
Fixes #12078
Added a visibility check on MudOverlay first render to avoid incorrectly decrementing the overlay lock count.