Fluentui: Fixing the modal Panel OuterClick issue

[frevds]

Test: https://codepen.io/frevd/pen/YgMJze

I want to be able to open a modal Panel, for which I set isBlocking to true (so that it shows an outer layer above the rest of the page for that the user cannot interact with it).

If isBlocking=true, the panel dismisses automatically whenever the user clicks on a foreign element (such as a dialog opened ontop, or another panel, anything that is not directly a DOM child of the panel).

If I prevent this using onOuterClick, the event get's cancelled - the issue is, if the event was a mousedown in a Textfield in a dialog ontop of the panel, then the mousedown get's cancelled, effectively rendering the Textfield unusable by mouse.

The real issue is the following code in Panel.base:

PanelBase.prototype._shouldListenForOuterClick = function (props) { return !!props.isBlocking && !!props.isOpen; };

IMO this must be corrected to return !props.isBlocking && !!props.isOpen, so that outerclicks are only observed if the panel is NOT modal. Everything else makes no sense, why would I want light-dismiss on a modal panel and no light-dismiss on a non-modal panel?

Unfortunately there is no other workaround but to create my own Panel class.
Would be great to have a fix for this in the future.
https://codepen.io/frevd/pen/YgMJze

The second issue that should be fixed is that the event gets cancelled (preventDefault) when an OnOuterClick-handler is provided on the Panel. The Panel has no authority to cancel DOM events such as mousedown, since it prevents totally unrelated UI elements from working correctly.

[frevds]

In case anybody needs a workaround right away, I had to make one because I need to display a modal panel and ontop of it a modal dialog with textboxes in it. If the user clicks in the dialog (or on its modal layer), the modal panel behind it closes. Specifying an OnOuterClick handler on the panel is not a solution, because this will cancel all mousedown-events, and users cannot use the mouse in the textboxes to select or set the cursor position). Not making the panel modal is also not a solution, as then the user can interact with the content behind it.

I tried to derive from the PanelBase class as a HOC, but when I do that the styles are missing. I copied the declaration of Panel, but I couldn't get it to work. To not spend more time on this issue I created a simple hack to change the PanelBase._shouldListenForOuterClick function directly, which needs to be executed somewhere before creating Panels and will solve the issue.

// Fix for Panel OuterClick issue (see https://github.com/OfficeDev/office-ui-fabric-react/issues/8470) // @ts-ignore PanelBase.prototype._shouldListenForOuterClick = (props: IPanelProps) => (!props.isBlocking && !!props.isOpen);

Update:
The above solution is too invasive and -see next comments- not desired.
Here is a longer but non-invasive extension as a workaround (proposed for a future version):
````
import { PanelBase } from 'office-ui-fabric-react/lib/components/Panel/Panel.base';
import { elementContains } from 'office-ui-fabric-react/lib/Utilities';

// Fix for Panel OuterClick issue (see https://github.com/OfficeDev/office-ui-fabric-react/issues/8470)
// Run this code once before opening any modal panel with external UI above it,
// to allow mouse-interaction in it and to have the panel stay open.
// @ts-ignore
PanelBase.prototype._dismissOnOuterClick = function (ev) {
var panel = this._panel.current;
if (this.state.isOpen && panel) {
if (!elementContains(panel, ev.target)) {
if (this.props.onOuterClick) {
if (this.props.onOuterClick() !== true) // unless the handler returns true
ev.preventDefault(); // default behaviour is to cancel the event on external UI
}
else {
this.dismiss();
}
}
}
};

// Then, when opening panels, specify the onOuterClick handler
// (to allow outer clicks without the panel closing) and return true (to keep the event):

````

[Vitalius1]

@frevds Thak you for submitting the issue along with a very detailed description of the problem and even better codepen demo.
@kkjeer you are listed as the code owner of the Panel, can you please take a look at the issue described by the author and see if it makes sense for the proposed solution?

[kkjeer]

@Vitalius1 I don't see any problems with the proposed solution!

[Vitalius1]

@frevds I remember the PR that introduced this behavior (#5422). Also, there was a similar issue as yours discussed in #7225 with an attempt to fix it in #7332. It was eventually agreed that the current behavior is somewhat correct and the proposed solution was to move the Dialog/Modal/Panel to be a child of Panel. Here are some comments in regards to this: Comment1, Comment2. I've modified your codepen here as proposed in the links above and indeed it solves the dismissal of the Panel.

The dismissOnOuterClick method along with the onOuterClick prop introduced in #5422 was solving the issue when the Panel in modal mode had its height modified within an app to not cover a Header with some actionable buttons in the top corner and when a user was clicking any of those (which were events from outside) we needed to control the dismissal of the Panel.

I remember an offline discussion why only for when it's in modal mode and the answer was if it's not blocking we should leave the parent component to listen and handle those events.
Your solution will cause anyone using a non-modal Panel and clicking outside (which is why I believe they chose a non-blocking Panel) to be forced to handle every such click.

As for the preventDefault() not sure why it's there 👀

[frevds]

@Vitalius1 - thanks for all the research.

If I could only control all the foreign elements the user can potentially click on, but I can not in general. Any page element that happens to show up above the panel but is not a child of it (maybe its from an unrelated or external component) kills the modal panel, so effectively the modal panel is not really modal. Now I am forced to handle outside clicks on any potential popup. This would be fine, but I'm left with three impossible choices: 1) if I attach that outerclick-handler, the popup GUI cannot receive mouse events, 2) if i don't attach the handler I loose the modal panel and its contents and can only reopen it (with an annoying blend-in animation) or 3) I have to guarantee that all DOM elements are children of the panel, which is close to impossible since I don't control all possible popups.

As for the preventDefault() not sure why it's there 👀

Well, it shall stop the outerclick, and it is very effective at that - the GUI that is deemed outer but is actually a popup ontop cannot receive the mousedown event.

Maybe we can argue about a better implementation of the handler then?

This is the current one (I only have the transpiled JS, sorry):
PanelBase.prototype._dismissOnOuterClick = function (ev) { var panel = this._panel.current; if (this.state.isOpen && panel) { if (!elementContains(panel, ev.target)) { if (this.props.onOuterClick) { this.props.onOuterClick(); ev.preventDefault(); // BAD, we should be able to decide that! } else { this.dismiss(); } } } };

I propose to allow to return true from the onOuterClick handler to indicate the event shall not be auto-prevented.
This is least invasive (all code will continue to work as before, and code aware of the new return value can utilize it to get popup GUI working again):

PanelBase.prototype._dismissOnOuterClick = function (ev) { var panel = this._panel.current; if (this.state.isOpen && panel) { if (!elementContains(panel, ev.target)) { if (this.props.onOuterClick) { if (this.props.onOuterClick() !== true) // unless the handler returns true ev.preventDefault(); // default behaviour is to cancel the event on external UI } else { this.dismiss(); } } } };

Would that be a possible way to go?

Update: I tested it. There are no other side effects with the above proposal (calling or not calling the ev.preventDefault). I adjusted my workaround and will update the above comment as well.

[Vitalius1]

@dzearing I would appreciate your input before proceeding with a PR. Thanks!

[dzearing]

We discussed offline, and believe that FocusTrapZone should have stopped propagation, but isn't getting a chance because Panel's _dismissOnOuterClick callback is listened with the "capture" bool. @Vitalius1 is testing the theory of getting rid of this so that FTZ can do the work correctly.

[ArrayKnight]

This seems related to: https://github.com/OfficeDev/office-ui-fabric-react/issues/8339

Would it be sufficient to simply not close the panel on blur if the isLightDismiss equals false?

PanelBase.prototype._shouldListenForOuterClick = function (props) {
    return !props.isBlocking && !props.isLightDismiss && !!props.isOpen;
}

[ArrayKnight]

@Vitalius1 @dzearing @kkjeer Also, is it possible to up the priority of this issue, please? This is creating a blocking bug for my MS project.

[frevds]

(I deleted my earlier comment from today about the issue having been solved - it is not)

The panel does not close when using the onOuterClick, that's fine, but if the foreign GUI contains an input field then the code in PanelBase eats the mouse event and one cannot click into the input field.

My workaround is to override this PanelBase method. For newer versions of Fabric (where this.state.isOpen does not exists any more and is now a private variable this._isOpen), my workaround needs to be adjusted as follows:

import { elementContains } from 'office-ui-fabric-react'; 
import { PanelBase } from 'office-ui-fabric-react/lib/components/Panel/Panel.base';

// Fix for Panel OuterClick issue (see https://github.com/OfficeDev/office-ui-fabric-react/issues/8470)
// @ts-ignore
PanelBase.prototype._dismissOnOuterClick = function (ev) {
    // @ts-ignore
    var panel = this._panel.current;
    // @ts-ignore
    var isOpen = this._isOpen; 
    if (isOpen && panel) {
        if (!elementContains(panel, ev.target)) {
            if (this.props.onOuterClick) {
                // @ts-ignore
                if (this.props.onOuterClick() !== true)  // unless the handler returns true
                    ev.preventDefault();  // default behaviour is to cancel the event on external UI
            }
            else {
                this.dismiss();
            }
        }
    }
};

[ksdaniel]

A week back we started receiving tickets that some of the forms in SPO, that we display in Panels (SPFX webparts), close as soon you click one of the controls.

Upon troubleshooting we noticed that regardless of where we click we trigger the outerclick handler.

I used the above workaround, and it fixed the issue - but I am a little bit worried, that I need to rewrite countless solutions now. Will open a new bug for this.

Any ideas what happened?

[yezidegithub]

@Vitalius1 My case is just like @frevds described that opening a panel containing input element with another one panel opened(must be opend), to prevent the former panel closed I use onOuterClick() but block the mouse event of input. The workaround work but I want to know if any idea update?

[Vitalius1]

@micahgodbolt can you please reassign the issue to whoever owns the control. Thanks!