Drawer routing and onClick issues when using with react.js

[aeon0]

I am using MDC with react.js and have an Issue with the mdc drawer. I striped down the react component to the basics:

import React from 'react';
import { NavLink, Link } from 'react-router-dom'

import * as mdc from 'material-components-web/dist/material-components-web';

class Header extends React.Component {
    constructor(props) {
        super(props);

        this.test = this.test.bind(this);
        this.open = this.open.bind(this);
    }

    componentDidMount() {
        this.drawer = new mdc.drawer.MDCTemporaryDrawer(this.refs.drawer);
    }

    test() {
        console.log("Test worked!");
    }

    open() {
        this.drawer.open = true;
    }

    render() {
        return (
            <header id="header" className="mdc-toolbar">

                <div className="mdc-toolbar__row">
                    <section className="mdc-toolbar__section mdc-toolbar__section--align-start">
                        <button onClick={() => this.open()}>open</button>
                        <button onClick={() => this.test()}>test</button>
                        <NavLink className="mdc-list-item" exact to="/dashboard">Dashboard Link</NavLink>
                        <Link className="mdc-list-item" to="/">Home Link</Link>
                    </section>
                </div>

                <aside className="mdc-temporary-drawer" ref="drawer">
                    <nav className="mdc-temporary-drawer__drawer">
                        <nav className="mdc-temporary-drawer__content mdc-list">
                            <NavLink className="mdc-list-item" exact to="/dashboard">Dashboard Link</NavLink>
                            <Link className="mdc-list-item" to="/">Home Link</Link>
                            <div className="mdc-list-item" onClick={() => this.test()}>Test</div>
                        </nav>
                    </nav>
                </aside>

            </header>
        );
    }
}
export default Header

Issue

The drawer opens and closes as expected. But the <NavLink/> and <Link/> are refreshing the page instead of routing without page refresh. Also, the onClick event calling this.test() is not working. Both things work as expected when putting them into <div className="mdc-toolbar__row">.
EDIT: The Issue occurs with Persistent & Temporary drawer. The Permanent drawer does not have these issues.

MDC-Web Version

material-components-web: 0.10.0
material: 0.1.1

Browser(s)

Chrome (Version 58.0.3029.96 (64-bit))
Firefox (53.0.2 (32-Bit))

OS

Win 10

[aeon0]

So It seems that the issue is caused by componentClickHandler calling stopPropagation(). I have this for a quick fix right now:

in material/drawer/slidable/foundation.js

// Comment or delete this  (Line 63)
//this.drawerClickHandler_ = (evt) => evt.stopPropagation();

in material/drawer/temporary/foundation.js

// Change this (Line 43)
this.componentClickHandler_ = (e) => this.close();
// To this
this.componentClickHandler_ = (e) => {
  if(e.srcElement.classList.contains("mdc-temporary-drawer"))
    this.close();
} 

Then importing like this:
import {MDCTemporaryDrawer, MDCTemporaryDrawerFoundation, util} from '@material/drawer';

I am not sure if this has any bad side effects on other behaviors of the drawer, so take it with a grain of salt.

Edit: had the same issue with the dialog component, too. To resolve this, I did pretty much the same but with the mdc-dialog__backdrop class.

[yeelan0319]

Hi @j-o-d-o , this is definitely a bug and it was reported in #579 as well.

I think your proposed solution make a lot of sense and it seems not changing the behavior for temporary and persistent drawer.

We would like to accept a PR for the fix if you are willing to help out! Basically it should include:

• Remove drawrClickHandler from /packages/mdc-drawer/slidable/foundation.js (line 63)
• Remove registerDrawerInteractionHandler that calls this method (line 93, line 100)
• Change componentClickHandler_ to the above suggested way.

  this.componentClickHandler_ = (e) => {
    if(e.srcElement.classList.contains("mdc-temporary-drawer"))
      this.close();
  } 
  

Thanks! This will help us unblock quite a few react.js users!

[aeon0]

#658 submitted a pull request, but need help on what to do to fix the unit tests which break because of deleting the click handler

[amsheehan]

@j-o-d-o did this actually work in your React implementation?

[amsheehan]

At its core, all react-router-dom's Link component does is pass a bunch of props down to an anchor tag. Perhaps what we're looking for here is Event.preventDefault() so the anchor tags that get rendered for Link don't force a page load when actioned on? Something like this is what I'm talking about:

handleClick(e) {
  e.preventDefault();
}

render() {
  return (
    <Link to='/bar' onClick={this.handleClick}>Bar</Link>
  );
}

@j-o-d-o can you toss up a codepen implementation of your use case so I can help resolve this issue? I think we need more work than just ripping out the click handlers in mdc-drawer/slidable/foundation.js

[aeon0]

That won't work because in my case the handleClick(e) function will never be called because of the stopPropagation() in the temporary drawer click handler.
The Solution I posted works for me. To also support firefox I had to change it to this:

let src = e.target || e.srcElement;
if(src.classList.contains("mdc-temporary-drawer"))
  this.close();

But that being said, I all with you! This is very hacky. Using a html class to determine what to do? Not that great imho. But just calling stopPropagation() on the drawer click handler does not seem like the right solution either. There already is a "drawerClickHandler_", so there is definitely a more elegant solution to this issue. I just needed a quick fix for my application.
The same issue is with dialogs. And especially in dialogs, a developer might want to have some user interaction apart from "decline" and "accept".

I will try to put up a codepen example.

[aeon0]

Okay, I looked at it again. What do you think about this possible solution?

in drawer/slideable/foundation.js

// add a class member flag
this.drawerClicked_ = false;

// set to true on click
this.drawerClickHandler_ = () => {
  this.drawerClicked_ = true;
}

in drawer/temporary/foundation.js which extends MDCSlidableDrawerFoundation

// use the flag on component click handler
this.componentClickHandler_ = () => {
  if(!this.drawerClicked_){
    this.close();
  }
  this.drawerClicked_ = false;
} 

[aeon0]

Alright, sorry for spamming here, but this didn't let me rest ;).

First off, two coderpens:

1. A vanilla javascript dialog example which is working as expected, the two buttons inside the dialog which are closing the dialog and firing a custom click event are working:
   http://codepen.io/j-o-d-o/pen/KmBRMo?editors=1010

2. A react.js example, same dialog, same buttons for closing and custom event. Not working:
   http://codepen.io/j-o-d-o/pen/rmrZZE

After some research on event order, capture & bubble phase. (https://w3c.github.io/uievents/) and how react.js is handling events...

Here is why this issue exists:
The Dialog and Component click handlers are both called in bubble phase (default behavior). That means all the event listeners "on top" of the dialog and component, weather called in capture or bubble phase are called beforehand. Thus, stopPropagation() inside the Dialog click handler is only affecting the component click handler and nothing else.
But react js is working differently. React.js registers a single event listener to it's root element and handles the rest internally. The event is registered with default behavior in bubble phase. That mean each event listener inside the root element is called before the react.js event listener fires! Now, in this case, the stopPropagation() in the Dialog click handler is affecting all reacht.js events.

[dimik]

Is it going to be fixed?
i also have step into this issue using React. Temporary workaround for me (without need to patch mdc) is listen on click on drawer element in capture phase and check if e.target is a navigation link so can call my onClick handler from here and stopPropagation

Full ugly code i have used:

  componentDidMount() {
    document.querySelector('.mdc-temporary-drawer').addEventListener(
      'click',
      this._fixClickHandler = e => {
        const target = e.target
        if (
          target.parentElement.tagName === 'NAV' &&
          typeof DRAWER_LINKS_ACTIONS[target.id] === 'function'
        ) {
          e.stopPropagation()
          DRAWER_LINKS_ACTIONS[target.id]()
        }
      },
      // use capturing event phase to fix
      true
    )
  }

  componentWillUnmount() {
    document.querySelector('.mdc-temporary-drawer')
      .removeEventListener('click', this._fixClickHandler)
  }

Can somebody suggest more simple solution here?

[RobertoAlvarezCeballos]

Facing the same issue.

@dimik Hi, I'm trying your solution but I don't fully understand what's DRAWER_LINKS_ACTIONS. Would you mind documenting that part a bit more?

Thanks for your solution.

[amsheehan]

Rolling this into #794 as that seems to be the root of the problem here as well. Please move conversation there. Closing.

[dimik]

@RobertoIDL DRAWER_LINKS_ACTIONS it's just a regular JS object to store event handlers by element id. Because you a not able to get event handler from e.target

const DRAWER_LINKS_ACTIONS = {
  '_nav_id_1_': (e) => {},
  '_nav_id_2_': (e) => {},
  '_nav_id_3_': (e) => {},
}

[aeon0]

Okay, now the dialog issue is finally fixed! Awesome! But now #794 and #644 (this one) are both closed while the issue explained here with the drawer still persists.

[Garbee]

Are you sure you're using the latest version of MCW?

[aeon0]

Using v0.16.0

[kfranqueiro]

This has been fixed via 3e173e1 (based on #1138), which refactors out the stopPropagation call.

[Aftab22]

Link to my code sandbox
Drawer with Items / buttons and working onclick method for each item

import React, { Component } from "react";
import { render } from "react-dom";
import AppBar from "material-ui/AppBar";
import MuiThemeProvider from "material-ui/styles/MuiThemeProvider";
import Drawer from "material-ui/Drawer";

import MenuItem from "material-ui/MenuItem";


class App extends Component {
  constructor() {
    super();
    this.state = {
      name: "React",
      open: false,
      pageNumber: 0
    };
    this.handleDrawerClick = this.handleDrawerClick.bind(this);
    this.handleMenuClick0 = this.handleMenuClick0.bind(this);
    this.handleMenuClick1 = this.handleMenuClick1.bind(this);
    this.handleMenuClick2 = this.handleMenuClick2.bind(this);
  }

  handleMenuClick0() {
    this.setState({ pageNumber: 0 });
    this.handleDrawerClick();
  }

  handleMenuClick1() {
    this.setState({ pageNumber: 1 });
    this.handleDrawerClick();
  }

  handleMenuClick2() {
    this.setState({ pageNumber: 2 });
    this.handleDrawerClick();
  }

  menuControl() {
    if (this.state.pageNumber == 0) {
      return (
        <div>
          <h1>
            Welcome , Please Click on the side bar to view the demo of react
            components.
          </h1>
        </div>
      );
    } else if (this.state.pageNumber == 1) {
      return (
        <div>
          <h1>Demo of Buttons</h1>
        </div>
      );
    } else if (this.state.pageNumber == 2) {
      return (
        <div>
          <h1>Demo Cards</h1>
        </div>
      );
    }
  }

  handleDrawerClick() {
    if (this.state.open == false) this.setState({ open: true });
    else this.setState({ open: false });
  }

  render() {
    return (
      <div>
        <MuiThemeProvider>
          <AppBar
            title="React Components"
            onLeftIconButtonClick={this.handleDrawerClick.bind()}
          />

          <Drawer open={this.state.open}>
            <MenuItem onClick={this.handleMenuClick0.bind()}>Home</MenuItem>
            <MenuItem onClick={this.handleMenuClick1.bind()}>Buttons</MenuItem>
            <MenuItem onClick={this.handleMenuClick2.bind()}>Cards</MenuItem>
          </Drawer>
        </MuiThemeProvider>
        {this.menuControl()}
      </div>
    );
  }
}

render(<App />, document.getElementById("root"));