Material-components-web: Components with open/close animations should have cancelable opening/closing start events

[jamesmfriedman]

This is more of a generalized issue and feedback regarding how MDC handles components that have an implied stateful lifecycle (opening, opened, closing, closed). Like most of my issues, this one is related to React, but is not React specific.

Because of the uni-directional data flow of React, interfacing with the components through RMWC is controlled via props. You don't execute an openMenu callback, you simply pass open=true. Again, following traditional React patterns, you would pass a callback to something like an onClose prop whenever the state is about to change to closed. This gives you the opportunity to respond to onClose and decided whether or not you want to close the component, or keep it opened. This a relatively standard pattern with what are called "controlled components".

A reductive example

class MyComponent extends React.Component {
  state = {
    open: false
  }

  render() {
    return (
      <Menu
        open={this.state.open}
        // maybe I want to stay open...
        onClose={() => this.setState({open: true})}
      />
    )
  }
}

The heart of where this becomes the problem is in the following code in the menu-surface foundation.

https://github.com/material-components/material-components-web/blob/af950fcc87a08db99ade9a8a100fe9d3e9de9f60/packages/mdc-menu-surface/foundation.js#L531

Basically, the MenuSurface has already started its un-cancellable close cycle before it notifies the consumers that it's going to close. This prevents consumers from doing something as simple as leaving the Menu open. The best I've been able to do in the upcoming RMWC release is to "re-open" the menu every time which gives you a nice little blip of the menu on the screen, as well as re-animate and re-call any logic when it opens (like focusing the first menu item).

This example is for the menu, but impacts anything that has similar opening / closing behavior (Menus, Dialogs, Drawers, and Snackbars at a quick glance).

Proposed Solution:

• Implement a standard pattern for components with an open / close lifecycle
• Keep an open: boolean flag in the foundation (Menu already has isOpen_)
• Wait a frame when trying to execute the close cycle and cancel if the open flag has been set back to true
• Dialog seems to be the closest fit to this pattern right now.

My ideal events

• open: executed immediately when the open changes from false to true
• opened: executed when all animations are finished and the component is resting. This may not execute if the component has been switched back to close.
• close: executed immediately when open changes from true to false
• closed: executed when all animations are finished and the component is resting. This may not be executed if the component has been switched back to open.

Sorry for the book. Thoughts?

[kfranqueiro]

What you're suggesting in terms of events sounds close to what we already have for dialog and drawer, but not menu-surface. We have opening/opened and closing/closed for those. We should at least consider mirroring that to menu-surface since it also animates.

I'm wondering a couple of things:

• What use cases do you have in mind for leaving the menu open? We tend to be wary of providing footguns to e.g. create something that's impossible to close (though technically we've kind of allowed that via configurable properties on dialog at this point).
• Is the "wait a frame" part absolutely necessary (at least for React, I guess)? i.e. what if we simply checked whether the event is canceled immediately after emitting it?

[jamesmfriedman]

• Its not actually about a specific use case, but more about controlled components in React. Components are expected to honor the props passed in to them. While it makes complete sense from accessibility and component design that you wouldn't want to leave the menu open, it just breaks the source of truth. The internal component (at least in React land) shouldn't be the source of truth about whether or not its open.
• The wait a frame isn't necessary, just don't know any other way to guarantee all of the code in the current call stack has executed. Reacts async rendering cycle optimizations are still a mystery to me TBH. Maybe not necessary, just pre-problem solving in my head?

On the footgun thing. Agreed. It's hard to factor how someone needs to use the components. A lot of it comes down to implementation details on the consumers end. No matter what, people can can use this library and still violate the material spec all day long (and I'm sure they will).

[kfranqueiro]

I think at the very least, we should apply the pattern we used for dialog and drawer to menu-surface, and consider making the events cancelable, at least on the same turn for starters. I'll icebox this for those purposes (and might re-title it to be more specific).

• [ ] Give menu-surface opening/opened/closing/closed events
• Make opening/closing events cancelable:
  
  
  
  • [ ] Dialog
  
  
  • [ ] Drawer
  
  
  • [ ] Menu-surface
  
  

[lucasecdb]

@moog16 I think this is related to material-components/material-components-web-react#785

[jamesmfriedman]

Definitely related. I understand the fear of following a controlled components pattern in some places because it will allow developers to do things that are non user friendly and out of spec (I.e. keep a menu open forever). However I don’t really know what the alternative is for integration into other frameworks.

[lucasecdb]

yeah, and it's exactly as you said before, it's impossible to control everything to keep aligned with the spec, and right now it's possible to violate it in a number of different ways. It should be responsibility of the user to keep these behaviours consistent.

[moog16]

@lucasecdb yes you're right - this is a similar issue. Although I think this is more of a React issue than a web issue in the controlled component context.

[moog16]

@lucasecdb I read what @dawsonc623 wrote, and I see what both of your are saying. We're looking into solutions. :)

[cpboyd]

If I set display: none on a dismissible or modal mdc-drawer, it seems to break the component's state.

I'm currently using a modal mdc-drawer on smaller layouts with a permanent mdc-drawer on larger ones.

I had simply been setting display: none on the relevant component, but it breaks any sort of animation.
If the underlying open() or close() get called, they get stuck in the animating state and the component never recovers (even after display is visible).