Describe the issue. What is the expected and unexpected behavior?
In Drawer component, when user opens the drawer and click outside the drawer, we hope the drawer will close automatically if it is not inline. However, we can only use toggle button or close icon to close it now.
Is this a bug or enhancement? If this issue is a bug, is this issue blocking you or is there a work-around?
Enhancement.
cc @jessiehuff
After discussing this with Jenn, one of the conclusions that she reached back at the beginning of working on a11y and PF components is that if a container can include tab-focusable contents and overlaps other contents, it should behave like a modal, and if it displays inline (or pushes contents over), it should behave like the disclosure pattern. That seemed to be the simplest rule-of-thumb that would avoid issues where the keyboard-only user loses sight of where the focus is (because it鈥檚 behind the contents that were made visible).
So, because the Drawer meets that rule of thumb of including tab-focusable contents and overlaps other contents, I could see it using the modal expectations (i.e. closing when clicking outside of it in this scenario). It might be worth asking designers whether the drawer that displays on top should behave like a popover (in terms of how we manage focus). The popover currently behaves like a modal, but additionally supports the behavior requested here.
@jessiehuff Thanks for your suggestions! At first I have the same idea, which make it behave like a modal when it is overlaps and make it behave like a disclosure pattern when it is inline. It's hard to implement all of this in the source code so I choose only to forward the ref prop for users to use in this PR #4587 . I also update the usage, it's more flexible. Could you please give me some suggestions if you have any idea about how to implement this inside the component? Thank you.
cc @mcarrano
I agree with @jessiehuff here, in addition to her reasoning I think it is safe to assume the user does not need to interact with the content that the panel is covering. In that case they should be using an inline panel, which is designed for the user to be able to be able to interact with both the panel content and the page content. This modal-like closing behavior will reinforce our intended use case for the component.
@mceledonia Thanks! So when the panel is inline, users could click everywhere and the panel will not close. When the panel is modal-like, it will close automatically when users click outside of it. Is that right?
@DaoDaoNoCode That is correct, it won't hurt to confirm with Matt once he's back next week.
@mceledonia @jessiehuff I agree with this for the most part with the following exception. In the case of a modal we display a modal screen over the page and access to elements in the layer below that are blocked until the user takes action on the modal. When the drawer overlays the page, the contents of the page below the drawer are still accessible. So in this scenario, if I try to click on something outside of the drawer, the drawer will close at the same time that focus is placed on the element, which could be confusing and/or not what the user intended. If we wanted the drawer to truly behave like a modal, we would block access to other elements on the page until the drawer closes.
@DaoDaoNoCode Can you say more about your use case and why you think we should make this change?
@mcarrano Thanks! In OpenShift, we put our notification drawer inside a drawer panel and make it overlap other contents. When we click on the notification badge, it will toggle the notification drawer. However, this is the only way to close the notification drawer, and what we want to do is to close it when user clicking outside it.
To implement the functions above, we need to add event listeners and reference of the drawer panel content to detect whether users click outside of it. At first, I want to do these in OpenShift. However, DrawerPanelContent in PatternFly is a React Functional Component and we cannot set a reference on it in OpenShift, so the simplest way is to forward the reference of the DOM node in DrawerPanelContent, so we can use it outside.
But this is not a user-friendly way, because user will need to create event listeners by themselves. But it's the most flexible way, for users could do anything they want if they can get the reference of the panel content.
This is why I think this function is important. Users who use our product sometimes have these needs. What's more, it's important how to implement it. We need to find a balance between user-friendly and flexibility. For example, we should't let users do a lot of things and creating a lot of functions when they are using this component, but we can provide an API named like onOutsideClick to define what users want to do when they clicking outside of the drawer panel. It will be both flexible and user-friendly.
We are currently adding a Close button to the drawer which was an oversight. I understand that in your case having the notification drawer close when the user clicks away from it makes sense. Just a bit wary to say that this should be the way the drawer works in all cases. @jcaianirh what are your thoughts? Do you think this behavior should be built into the PatternFly component?
@mcarrano I'd like to see what patternfly would recommend, but thinking through the openshift specific use case. We actually have the possibility of having multiple drawers open at the same time...so what would an outside click do? Close all drawers? Maybe a close button would be the best solution to this use case...do any other products have input or experience with this ?
@jcaianirh We do have an issue open here for adding the close button to the Notification drawer https://github.com/patternfly/patternfly-react/issues/4201 This is due to be addressed in the 2020.12 release. @DaoDaoNoCode would that solve the problem for you? I am OK with also making this change to allow clicking away from the drawer a way to close it, but that should not be the default behavior as it does get complicated in certain use cases. Thoughts?
@mcarrano Thank you, I agree with @jcaianirh that adding the close button is enough to solve the problem.
In that case I will close this. Thanks @jcaianirh @DaoDaoNoCode