MudDrawer: Fix initialization behaviour, use ParameterState, remove PreserveOpenState by ScarletKuro · Pull Request #8833 · MudBlazor/MudBlazor

ScarletKuro · 2024-04-29T02:11:00Z

Description

Replaces PR: #5012
Fixes: #4585
Fixes: #7273
Fixes: #4535

If you open the docs in small initial size, and start to resize it (large direction), the Drawer will not open. This should fix it.
Added our ParameterState framework.
I removed PreserveOpenState, it doesn't seems useful anymore, it looks like #684 (comment) it was just some property to overcome some implementation problems.
Now when the _isOpenWhenLarge is gone (it was the main culprit why this whole thing wouldn't work adequately) the PreserveOpenState is the default behavior now because it stays open when you increase the browser size:

MudBlazor/src/MudBlazor/Components/Drawer/MudDrawer.razor.cs

Lines 317 to 321 in fa7d7e8

    
           else if (ShouldOpenDrawer(breakpoint)) 
        
           { 
        
               await _openState.SetValueAsync(true); 
        
               isStateChanged = true; 
        
           }

As I understand the _isOpenWhenLarge existed to restore the state when you lower window from a large window size and then resize it back then it goes to the "original" state before the resize. For example, if you had a large window and drawer closed (manually), then make the window small and back large again the drawer doesn't open, the PreserveOpenState was there to disable this this restore logic, but it doesn't help with initial behavior. This in fact all conflicts with the initial behavior when you start with a small window, the _isOpenWhenLarge is triggering when you resize to small window, but you already started small so it's just doesn't do anything. I removed all this logic, as it hard for me to find the justification for this default behavior, and there were no test coverage for it, all responsive tests were done against PreserveOpenState=true.
The logic is simple, if it doesn't meet the breakpoints condition the drawer closes, and it does it opens, no "smart" features like restore state from large window etc.

I tried to chunk the code to small methods so that the main logic would look like this:

if (ShouldCloseDrawer(breakpoint))
{
	await _openState.SetValueAsync(false);
}
else if (ShouldOpenDrawer(breakpoint))
{
	await _openState.SetValueAsync(true);
}

Also it seems like the missing popover provider was the reason why examples wouldn't work locally in Edge as I complained in discord.

How Has This Been Tested?

new bUnit tests Visual

Type of Changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation (fix or improvement to the website or code docs)

Checklist

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

…reserveOpenState

ScarletKuro · 2024-04-29T02:15:11Z

Would be nice, if someone is willing to test it with their drawers.

codecov · 2024-04-29T02:19:30Z

Codecov Report

Attention: Patch coverage is 84.09091% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 90.13%. Comparing base (28bc599) to head (fa7d7e8).
Report is 127 commits behind head on dev.

Files	Patch %	Lines
src/MudBlazor/Components/Drawer/MudDrawer.razor.cs	83.33%	4 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8833      +/-   ##
==========================================
+ Coverage   89.82%   90.13%   +0.30%     
==========================================
  Files         412      421       +9     
  Lines       11878    12277     +399     
  Branches     2364     2429      +65     
==========================================
+ Hits        10670    11066     +396     
+ Misses        681      662      -19     
- Partials      527      549      +22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

henon

Moving the boolean expressions into their own methods resulted in much more readable code. Good job!

henon · 2024-04-29T08:17:24Z

Added to v7.0.0 Migration Guide #8447

digitaldirk · 2024-05-09T21:49:08Z

Just upgraded to 7 preview 2 and now when I toggle light/dark mode my drawer closes. I believe this pr is the change that is causing this.

I will try to make a repo to show the issue @ScarletKuro

ScarletKuro · 2024-05-09T22:24:09Z

I will try to make a repo to show the issue @ScarletKuro

Sure, we will wait for repo, as I couldn't reproduce it on dev.mudblazor.com (it uses latest changes) that also has a drawer and dark/light switch.
It's weird since Drawer doesn't eve know anything about the light / dark theme, so it could be something else.

digitaldirk · 2024-05-09T23:04:06Z

@ScarletKuro Thanks for looking into this.

It seems to be related to the Breakpoint="Breakpoint.SmAndDown" property. When that is removed it works normally. After this issue occurs, if you reopen the drawer (takes 2 clicks to do so), then changing the theme doesn't close the drawer. I also noticed if you resize the browser or fullscreen you will encounter similar issues.

Issue demo:

Repo:
https://github.com/digitaldirk/DarkLightModeMudDrawerIssue

My real project (which is not public) has a mud drawer setup more like this, but I removed that complexity from the demo repo:

  <MudHidden Breakpoint="Breakpoint.SmAndDown">
    <MudDrawer Class="px-0" Elevation="5" Open="_drawerOpen" Breakpoint="Breakpoint.SmAndDown" Variant="@DrawerVariant.Persistent" Overlay="false" ClipMode="DrawerClipMode.Always">
      <NavMenu />
    </MudDrawer>
  </MudHidden>

  <MudHidden Breakpoint="Breakpoint.MdAndUp">
    <MudDrawer Elevation="5" Class="pa-0 ma-0" MiniWidth="50px" Fixed="true" OpenMiniOnHover="true" Variant="@DrawerVariant.Mini" Overlay="true" ClipMode="DrawerClipMode.Always">
      <NavMenu />
    </MudDrawer>
  </MudHidden>

ScarletKuro · 2024-05-10T09:37:05Z

Hi,

I've reviewed your repo, and I believe it now functions as expected. The previous behavior seemed incorrect.

There are two main issues with your code:

Not utilizing bind-Open for the drawer.
Using SmAndDown as the Breakpoint in the drawer (this is more of a bug than a feature, but let me explain).

It seems to be related to the Breakpoint="Breakpoint.SmAndDown"

It may not be easy to explain, but I'll start with the Breakpoint. Though it's not documented, in the current MudDrawer code, SmAndDown is considered "illegal". It only working with Xs, Sm, Md, Lg, Xl, and Xxl, unlike MudHidden.

Even before my changes, the drawer's breakpoint comparison was using MudDrawer.Breakpoint < currentBreakpoint (converting enum to int) instead of IsBreakpointWithinReferenceSizeAsync(currentBreakpoint, MudDrawer.Breakpoint). This means that the code always returned that the breakpoint was below (for SmAndDown and alike breakpoints) the needed value, indicating that the drawer should be in a closed state.

Previously, the drawer wasn't closing because the initial behavior wasn't functioning correctly. Since you're using a Persistent drawer, it won't react to resize changes and it gets unnoticed that drawer never changes its state. Now, with the understanding that the drawer always wants to close itself with SmAndDown, here's what it's happening next:

Since you're not using two-way binding, the drawer wants to close, but you have a variable with an initial value of "true" keeping the drawer open. However, when you click the theme toggle, it internally calls StateHasChanged, causing the internal state of the drawer to switch to its correct state, which is closed.

So the main reasons it was working before are:

The initial drawer state never worked correctly.
Our code was overriding the Open setter and getter, interfering with the parent-child communication.

If you change to @bind-Open="_drawerOpen" and Breakpoint="Breakpoint.Sm", it will function as intended. With the current code, Sm means that all breakpoints from Sm downwards will close the drawer.
Based on your configuration, the intended behavior should be as follows: If it's Sm or below, the drawer should be closed; if it's above Sm, it should be open. Since it's persistent, it should not react to any subsequent resize changes. And this is what it will do if you fix this two things.

Perhaps we should consider making changes from our side to handle SmAndDown properly by using the IsBreakpointWithinReferenceSizeAsync method or throwing an exception indicating that it shouldn't be used.

ScarletKuro · 2024-05-10T16:39:21Z

I added a PR #8941 that makes Drawer to understand to work not only with Xs, Sm, Md, Lg, Xl, Xxl

digitaldirk · 2024-05-10T17:54:25Z

@ScarletKuro
Thank you for the detailed explanation, that helped me understand the breakpoints and drawers better. Fixed my issue.

MudDrawer: Fix initialization behaviour, use ParameterState, remove P…

fa7d7e8

…reserveOpenState

github-actions bot added breaking change This change will require consumer code updates bug Unexpected behavior or functionality not working as intended PR: needs review labels Apr 29, 2024

ScarletKuro requested review from danielchalmers, henon and mikes-gh April 29, 2024 02:14

henon approved these changes Apr 29, 2024

View reviewed changes

henon added API change Modifies the public API surface v7 and removed PR: needs review labels Apr 29, 2024

henon mentioned this pull request Apr 29, 2024

v7.0.0 Migration Guide #8447

Closed

henon merged commit 8dee6ef into MudBlazor:dev May 2, 2024

ScarletKuro mentioned this pull request May 10, 2024

MudDrawer: Improve breakpoint logic #8941

Merged

7 tasks

ScarletKuro mentioned this pull request Jul 3, 2024

MudDrawer opens on its own after upgrading to V7 #9307

Closed

2 tasks

ScarletKuro mentioned this pull request Nov 27, 2024

Drawer open when resize browser window #10339

Closed

4 tasks

dependabot bot mentioned this pull request Aug 7, 2025

chore(deps): Bump MudBlazor from 6.13.0 to 7.16.0 manuel-rw/grocy-scanner#36

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MudDrawer: Fix initialization behaviour, use ParameterState, remove PreserveOpenState#8833

MudDrawer: Fix initialization behaviour, use ParameterState, remove PreserveOpenState#8833
henon merged 1 commit intoMudBlazor:devfrom
ScarletKuro:drawer_state

ScarletKuro commented Apr 29, 2024 •

edited

Loading

Uh oh!

ScarletKuro commented Apr 29, 2024

Uh oh!

codecov bot commented Apr 29, 2024

Uh oh!

henon left a comment

Uh oh!

henon commented Apr 29, 2024

Uh oh!

digitaldirk commented May 9, 2024

Uh oh!

ScarletKuro commented May 9, 2024 •

edited

Loading

Uh oh!

digitaldirk commented May 9, 2024

Uh oh!

ScarletKuro commented May 10, 2024 •

edited

Loading

Uh oh!

ScarletKuro commented May 10, 2024

Uh oh!

digitaldirk commented May 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	else if (ShouldOpenDrawer(breakpoint))
	{
	await _openState.SetValueAsync(true);
	isStateChanged = true;
	}

Uh oh!

Conversation

ScarletKuro commented Apr 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Type of Changes

Checklist

Uh oh!

ScarletKuro commented Apr 29, 2024

Uh oh!

codecov bot commented Apr 29, 2024

Codecov Report

Uh oh!

henon left a comment

Choose a reason for hiding this comment

Uh oh!

henon commented Apr 29, 2024

Uh oh!

digitaldirk commented May 9, 2024

Uh oh!

ScarletKuro commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

digitaldirk commented May 9, 2024

Uh oh!

ScarletKuro commented May 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ScarletKuro commented May 10, 2024

Uh oh!

digitaldirk commented May 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ScarletKuro commented Apr 29, 2024 •

edited

Loading

ScarletKuro commented May 9, 2024 •

edited

Loading

ScarletKuro commented May 10, 2024 •

edited

Loading