Skip to content

MudOverlay: Add parameter Modal allowing click-through#10893

Merged
danielchalmers merged 55 commits into
MudBlazor:devfrom
Cybrosys:feature/mudmenu-modal-toggle
Apr 1, 2025
Merged

MudOverlay: Add parameter Modal allowing click-through#10893
danielchalmers merged 55 commits into
MudBlazor:devfrom
Cybrosys:feature/mudmenu-modal-toggle

Conversation

@Cybrosys

@Cybrosys Cybrosys commented Feb 17, 2025

Copy link
Copy Markdown
Contributor

Description

Introduces a new opt-in behavior that allows pointer events to pass through, enabling users to interact with other elements on the page directly. The overlay will close upon interaction with other elements if AutoClose is true, eliminating the need for an extra dismiss click/tap.

The Modal parameter has been introduced to several components that currently use MudOverlay to handle dismissal or toggling between open and closed states, such as menus and picker controls, this to achieve the expected behavior while not blocking interaction with elements behind the overlay.

#10738

Extra: #10938

How Has This Been Tested?

Unit and visually

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.

@github-actions github-actions Bot added docs Changes to project docs site that do not affect core library logic enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library labels Feb 17, 2025
@codecov

codecov Bot commented Feb 17, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.01%. Comparing base (11a8013) to head (380f4ee).
Report is 3 commits behind head on dev.

Files with missing lines Patch % Lines
...Services/PointerEvents/PointerEventsNoneService.cs 89.47% 1 Missing and 1 partial ⚠️
...c/MudBlazor/Components/Overlay/MudOverlay.razor.cs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10893      +/-   ##
==========================================
+ Coverage   90.99%   91.01%   +0.01%     
==========================================
  Files         431      436       +5     
  Lines       14055    14095      +40     
  Branches     2722     2724       +2     
==========================================
+ Hits        12790    12829      +39     
- Misses        650      651       +1     
  Partials      615      615              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@versile2

Copy link
Copy Markdown
Contributor

I thought we were talking about putting it on the Overlay so it would apply to all controls that use the overlay instead of just mudmenu

@Cybrosys

Copy link
Copy Markdown
Contributor Author

I thought we were talking about putting it on the Overlay so it would apply to all controls that use the overlay instead of just mudmenu

Ah, my misunderstanding! I’ll look into shifting it to the Overlay.

@Cybrosys

Cybrosys commented Feb 18, 2025

Copy link
Copy Markdown
Contributor Author

I'm struggling to implement this in MudOverlay because using pointer-events: none prevents the overlay from being returned when using elementsFromPoint() or similar methods to determine which element is directly underneath the cursor. My approach was to check if MudOverlay was the topmost element or not, but since it is ignored by elementsFromPoint(), this doesn't work as expected.

In the current PR, the logic checks whether the clicked event is inside any div with the class mud-menu-list. I could likely query elements at a specific point as well. If the click occurs inside an element with this class, the menu remains open.

At this point, the only approach I see is for the component using MudOverlay to explicitly provide a class or identifier that MudOverlay can query and check against (for example, MudMenu would provide mud-menu-list).

I'm open to any other suggestions or insights on how best to approach this. Let me know what you think!

Update
Going by the MudMenu example, I can see there is an element under the cursor with the class mud-popover. I am however not sure this will be the case for the other components.

@versile2

Copy link
Copy Markdown
Contributor

so there are only a handful of popover / overlay combo controls (autocomplete, select, picker, menu?) so it's easy to look at each one. I think the only reasonable way I could think about it would be to

a) create a css class on the overlay something like mud-overlay-allow-clickthrough then in mudPopoverJs where I'm adjusting the overlay css you have access to the popover element ID etc so it would be easy to ignore it on a click handler. If that class exists then do that, etc.

b) create a separate component and add Overlay true/false default to true on the components that use it, then anyone who wanted overlay functionality with clickthrough could use the new component after setting Overlay=false on the popover component. I'm unsure of this one but it's an idea that went through my head.

c) create a js item that is popover agnostic and accepts an element. In each of the popover/ overlay components add a property OverlayClickThrough that when true onafterrender will attach the element to the js function. On the overlay create a method that does that allowing the dev to pass the element to the Overlay.

And before I did any of those I'd wait for @danielchalmers to chime in, he may hate/love one over the other or decide not to move forward with it. That way if you do the work at least your pretty sure it will be accepted.

@Cybrosys

Cybrosys commented Feb 19, 2025

Copy link
Copy Markdown
Contributor Author

The core issue is that once we use pointer-events: none to allow interaction through the overlay (and also show the correct mouse cursor), the overlay no longer receives any click events. JavaScript does not support forwarding click events or generating new ones.

The fourth solution d) would be to add a new css class mud-overlay-block-clickthrough on elements that should block passthrough. For the MudMenu this would be the mud menu items container. I would also add and remove the css class conditionally on the activator element. This would allow the MudOverlay JS to check if there are any elements under the pointer with the css class or not. If no elements have the class then it calls a method on the dotnetref (MudOverlay).

Letting each component add and remove the css class to elements would solve the search panel problem that Daniel encountered. Why conditionally add/remove? That is if you have two search panels next to each other, you want interaction with the other one to close the overlay from the first if shown.

So the css class on the activator element would be conditionally added when the overlay is shown and removed when closed.

@danielchalmers

Copy link
Copy Markdown
Member

I apologize, I can't suggest a firm direction right now. The mud-overlay-allow-clickthrough example (solution a+d) seems logical since @versile2 has used that technique a few times, but I'll defer to him as he is experienced in this

@danielchalmers

Copy link
Copy Markdown
Member

I'll add that MUI is an interesting example: Autocomplete is non-modal, but Select and Menu are.

@Cybrosys

Cybrosys commented Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

I apologize, I can't suggest a firm direction right now. The mud-overlay-allow-clickthrough example (solution a+d) seems logical since @versile2 has used that technique a few times, but I'll defer to him as he is experienced in this

Sorry, re-adding my comment. I thought I was onto something regarding how MudOverlay was currently preventing interaction but it was merely an effect of z-index and absolute positioning.

Original comment:
I'm not fully following how mud-overlay-allow-clickthrough should or could be used. As I mentioned, using pointer-events: none allows the cursor to interact with elements behind the MudOverlay. However, this also causes the overlay to be excluded from interaction events, meaning it won't be returned when querying elements under the cursor, and it will no longer receive any click events.

From what I've found through research and testing, a reliable solution should focus on the element(s) containing the popup content. Using the MudMenu example, the root element of the menu items would serve as the reference. A global click event would then check if the element under the cursor is a child of the menu’s root element. If it's not, the click is considered outside the popup, and the menu is dismissed. However, as that is not fully reliable I would still opt for the use of a CSS class for the modal elements (menu items).

In this context, MudOverlay seems to primarily serve the purpose of preventing interaction with background elements (true modal behavior) and optionally shading the background.

That said, I might have misunderstood the intended usage of mud-overlay-allow-clickthrough, so I appreciate any further clarification.

@versile2

Copy link
Copy Markdown
Contributor

Just a blind one eye pass on it from behind the keyboard not running the code.

so on the click handler of the mudoverlay it would check if that class exists if it does it would pass it's elementreference and event (the clientX and clientY of mouseeventargs) to a javascript function at the end of the click handler. So it completes the overlay purpose then runs the following javascript function.

focusElementUnderOverlay(element, event) {
// make sure overlay valid
if (!element) return;
// get element underneath
const elementUnder = document.elementFromPoint(event.clientX, event.clientY);

// pass through
if (elementUnder) {
elementUnder.click(); // might should be focus?
}
}

@Cybrosys

Cybrosys commented Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

For that to potentially work we would not be able to use pointer-events: none. If we are not using it then you won't see any visual feedback as the mouse goes over different elements, elements won't highlight and the cursor won't change.

If you run the docs server in this PR and see the updates docs example for the Menu, you can see how the cursor reacts and interacts with the elements differently depending on if the "Modal Menu" or "Non-modal Menu" is showing.

Another caveat would be if it is even possible to alter the click event or simulate a new one. I think I have tried that in the past but those events are ignored as they are considered a security risk.

@versile2

Copy link
Copy Markdown
Contributor

then the flip side is onafterrender of mudoverlay you remove the click handler if that class exists, run your js, at the click of your js execute the close handler of the mudoverlay from dotnetref. In your js if the item you click has the same or higher zindex then you do nothing. Again spitballing I'm not coding some or all of this might not work like we want.

@Cybrosys

Cybrosys commented Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

then the flip side is onafterrender of mudoverlay you remove the click handler if that class exists, run your js, at the click of your js execute the close handler of the mudoverlay from dotnetref. In your js if the item you click has the same or higher zindex then you do nothing. Again spitballing I'm not coding some or all of this might not work like we want.

I tried checking the z-index of the clicked element to try and compare it with the z-index of the mud overlay, to see if it's in front of it or not, but for some reason the clicked element's z-index was always lower than the overlay's. I might not have done it properly, so might be worth a retry.

@versile2

Copy link
Copy Markdown
Contributor

then the flip side is onafterrender of mudoverlay you remove the click handler if that class exists, run your js, at the click of your js execute the close handler of the mudoverlay from dotnetref. In your js if the item you click has the same or higher zindex then you do nothing. Again spitballing I'm not coding some or all of this might not work like we want.

I tried checking the z-index of the clicked element to try and compare it with the z-index of the mud overlay, to see if it's in front of it or not, but for some reason the clicked element's z-index was always lower than the overlay's. I might not have done it properly, so might be worth a retry.

aye let me know if you have a code set that's close I'll take a look. Might be some other markers too, if they are all popovers then traverse to see if data-ticks is greater than 0, if it is it's an active popover and should be ignored.

@Cybrosys

Copy link
Copy Markdown
Contributor Author

then the flip side is onafterrender of mudoverlay you remove the click handler if that class exists, run your js, at the click of your js execute the close handler of the mudoverlay from dotnetref. In your js if the item you click has the same or higher zindex then you do nothing. Again spitballing I'm not coding some or all of this might not work like we want.

I tried checking the z-index of the clicked element to try and compare it with the z-index of the mud overlay, to see if it's in front of it or not, but for some reason the clicked element's z-index was always lower than the overlay's. I might not have done it properly, so might be worth a retry.

aye let me know if you have a code set that's close I'll take a look. Might be some other markers too, if they are all popovers then traverse to see if data-ticks is greater than 0, if it is it's an active popover and should be ignored.

I did a test and the z-index was not reliable. I found some people on SO who had tried to solve it but it escalated into someone writing code that mimics/builds the visual hierarchy of the entire page with JS, let's not use that.

Analyzing the DOM hierarchy, everything in front of the mud overlay will be a child under a popovercontent-div. The parent divs could be identified by the data-ticks attribute, but also the CSS classes mud-popover and maybe mud-popover-open. The mud overlay is at the same DOM depth as the popovercontent-divs.

We could solve it by either:
a) Calculating the DOM depth of the clicked element and mud overlay, and comparing them.
b) Traverse up the DOM tree from the clicked element until we find a div with data-ticks > 0 and/or one that has the CSS class(es) mud-popover

I'll try and prep a trymud or sample with a and b to choose between.

@versile2

Copy link
Copy Markdown
Contributor

sounds good, you can use javascript on https://try.arctechonline.tech if needed

@Cybrosys

Cybrosys commented Feb 22, 2025

Copy link
Copy Markdown
Contributor Author

sounds good, you can use javascript on https://try.arctechonline.tech if needed

Here's a thrown together TryMudBlazor snippet based on some of my findings and tests.

Key Findings:

  • Using DOM depth to determine if an element is behind the overlay was unreliable. Elements behind the overlay could have varying DOM depths depending on page content, making it unrelated to the overlay's depth.

  • The sample includes:

    1. A modal menu
    2. A popover toggle button
    3. A non-modal menu (with pointer-events: none on the overlay)
  • The popover demonstrates how components using MudPopoverProvider can produce false positives.

  • MudDialog doesn’t use MudPopoverProvider, which is worth noting.

JavaScript Methods:

  1. isTargetBehindOverlay:

    • Checks if the clicked target (or its ancestor) shares the same parent as MudOverlay.
    • Works for both components using MudPopoverProvider and those that don’t, like MudDialog.
  2. isTargetPopoverContent (less reliable):

    • Requires MudPopoverProvider.
    • Uses element.closest() to check if the element or its ancestors have the mud-popover-open CSS class.

Additional Suggestion:

  • Another possible approach would be to introduce new CSS classes specifically for identifying elements that should not cause the overlay to be dismissed, providing more granular control when a non-modal behavior is desired.
    • ... or pass the @ref of the root element of the elements that should not cause it to be dismissed to the javascript. But this also requires changes to all components that wish this behavior/feature.
  • In my local branch I am controlling the CSS style on the MudOverlay using a parameter called Modal that is also exposed on other components such as the MudMenu (<MudMenu Modal="false">).

@Cybrosys

Cybrosys commented Feb 22, 2025

Copy link
Copy Markdown
Contributor Author

@versile2 @danielchalmers Persistence pays off. I had an idea and just tested it, successfully. There is a way to use pointer-events: none and get the element using elementsFromPoint. The solution is to change the style of the MudOverlay before calling the method and then restore it:

isOverlay(event) {
    const overlay = document.querySelector(".mud-overlay");
    if (!overlay) {
        console.log("Overlay not found, ignoring.");
        return false;
    }

    // Temporarily enable pointer events
    overlay.style.pointerEvents = "auto";

    const elementsFromPoint = document.elementsFromPoint(event.clientX, event.clientY);

    // Disable pointer events again
    overlay.style.pointerEvents = "none";

    if (elementsFromPoint.length > 0 && elementsFromPoint[0] === overlay) {
        return true;
    }

    return false;
 }

Here's a TryMud snippet resembling the previous one.

@Cybrosys

Copy link
Copy Markdown
Contributor Author

There might be a layout race condition, but you can force a synchronous layout reflow by accessing a property immediately after changing the style. I didn’t need to do this during my tests, but if needed, it would look like this:

overlay.style.pointerEvents = "auto";
overlay.offsetHeight; // Trigger reflow

@versile2

versile2 commented Feb 22, 2025 via email

Copy link
Copy Markdown
Contributor

@versile2

Copy link
Copy Markdown
Contributor

ok so looks good and what is required to make this a fairly generic thing for overlay? Such as an option on Overlay for Modal true/false or do we have to add a property on everything that uses an Overlay. Walk me through it, but I like it and apart from some testing I can't see it not working right.

@Cybrosys

Cybrosys commented Feb 24, 2025

Copy link
Copy Markdown
Contributor Author

The term Modal is well-known in app development, referring to an element that appears in front of others and blocks interaction with the background. As such I don't feel that Modal = true/false is a bad choice for the MudOverlay. If we look at some other parameters that activate features, maybe the parameter should be called Modeless.

I believe that the current default behavior (Modal = true) has advantages for touch-based interfaces. So whether this should be the default behavior is worth discussing with a broader audience, as there may be unexpected side effects or differing opinions.

If we keep to having the current behavior the default, so this would be "opt-in". I would then suggest adding the same parameter (Modal) to controls that use the MudOverlay for dismissing popup elements and bind it. This means that to be able to interact with the background while a MudMenu is showing would look something this:

<MudMenu Modal="false">...</MudMenu>

or

<MudMenu Modeless>...</MudMenu>

This feature also ties in to AutoClose on MudOverlay. So the requirement for activating the JavaScript listener is that AutoClose is true and Modal is false.

@Cybrosys

Cybrosys commented Feb 24, 2025

Copy link
Copy Markdown
Contributor Author

Maybe add Modeless or Modal to MudGlobal (or some other place) so it's possible to opt-in globally for the new behavior?

@versile2

versile2 commented Feb 24, 2025

Copy link
Copy Markdown
Contributor

We won't purposefully introduce a breaking change outside of major releases so we must keep current behavior or shelve this until we are closer to V9.

I like Modal true/false over Anything else but I also don't like having to put it on each item that uses Overlay. Also exclude Dialogs and Drawer. Those aren't our targets for this, select, autocomplete, menu are the top 3. And some way to trigger from the Overlay itself? public method SetModelessElement(ElementReference element) or something like that?

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

Labels

enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants