MDL Ripple component is created/initialized before custom component.

leifoolsen commented 8 years ago

MDL version: 1.1.2 Browser: Chrome Browser version: 48.0.2564.103 (64-bit) Operating system: OS-X Operating system version: 10.11.3 URL: NA What steps will reproduce the problem:

1. I'm trying to write an MDL-component with ripple effect on child elements, similar to the MDL tabs component. The problem I am experiencing is that it seems as if MaterialRipple is initialized before I get the chance to add the mdl-js-ripple-effect--ignore-events class, and thus prevent the triggered ripple events to fire for the container.

It may be that this is not an issue, but rather something I've missed when it comes to developing MDL components.

2. Html to support component

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>A basic MDL component</title>
  <link rel="stylesheet" href="../node_modules/material-design-lite/material.css" />
</head>
<body>
  <div id="mount">
    <div class="mdl-basic mdl-js-basic mdl-js-ripple-effect">
      <p>My basic MDL component</p>
    </div>
  </div>
  <script type="text/javascript" src="../node_modules/material-design-lite/material.js" charset="utf-8"></script>
  <script type="text/javascript" src="./mdl-basic-component.js" charset="utf-8"></script>
</body>
</html>

3. The stripped down component:

(function() {
  'use strict';

  const MaterialBasic = function MaterialBasic(element) {
    // Stores the element.
    this.element_ = element;

    console.log(this.element_.classList, 'data-upgraded', this.element_.getAttribute('data-upgraded'));

    // Initialize instance.
    this.init();
  };
  window['MaterialBasic'] = MaterialBasic;

  MaterialBasic.prototype.Constant_ = {
    RIPPLE_COMPONENT: 'MaterialRipple'
  };

  MaterialBasic.prototype.CssClasses_ = {
    IS_UPGRADED: 'is-upgraded',
    JS_RIPPLE_EFFECT: 'mdl-js-ripple-effect',
    JS_RIPPLE_EFFECT_IGNORE_EVENTS: 'mdl-js-ripple-effect--ignore-events'
  };

  MaterialBasic.prototype.init = function() {
    if (this.element_) {
      if (this.element_.classList.contains(this.CssClasses_.JS_RIPPLE_EFFECT)) {
        // Ignore ripple event on this container 
        this.element_.classList.add(this.CssClasses_.JS_RIPPLE_EFFECT_IGNORE_EVENTS);
      }

      // Do the init required for this component to work

      // Set upgraded flag
      this.element_.classList.add(this.CssClasses_.IS_UPGRADED);
    }
  };

  // The component registers itself. It can assume componentHandler is available
  // in the global scope.
  /* eslint no-undef: 0 */
  componentHandler.register({
    constructor: MaterialBasic,
    classAsString: 'MaterialBasic',
    cssClass: 'mdl-js-basic'
  });
})();

What is the expected result? Ripple event should not fire on container

What happens instead of that? Ripple event fires and throws an error: "Uncaught TypeError: Cannot read property 'classList' of null"

If I add the class mdl-js-ripple-effect--ignore-events in markup everything works as expected.

I would expect that my component was initialized before the MDL Ripple component, but it does not seem to happen. Is there a known workaround for this?

Regards Leif Olsen

vmateixeira commented 8 years ago

@leifoolsen I tried to reproduce your issue by using your MaterialBasic component, both in Firefox (x64) 47.0a2 (2016-03-16) and Chrome 41.0.2272.118, under Windows 7 (x64) but was unsuccessful. :pensive:

leifoolsen commented 8 years ago

If unsuccessful means tat the ripple event did not fire, the you were successful :-)

Source code for this case: html: https://github.com/leifoolsen/mdl-ext/blob/master/demo/mdl-basic-component.html js: https://github.com/leifoolsen/mdl-ext/blob/master/demo/mdl-basic-component.js

Just click on the text "My basic MDL component" and watch the console log.

leifoolsen commented 8 years ago

@vmateixeira: I suspect what may be causing this: In gulpfile.babel.js ripple.js is the final component that loads. My component, which is a third party component, loades after ripple.js. This may be a possible cause of the problem I'm experiencing.

vmateixeira commented 8 years ago

My bad.. guess after all I'm still unsuccessful :disappointed:

snip

Although, this doesn't seem to happen with any existing component with ripple... For instance, on a button:

snip2

leifoolsen commented 8 years ago

@vmateixeira: Thanks for looking into this. You're getting excactly the output in console that I reported. I think the cause of this is that Ripple is initialized before my (3'rd party) component.

dgrubelic commented 8 years ago

Thus the comment...

// And finally, the ripples line ~90 of gulpfile.babel.js

leifoolsen commented 8 years ago

So conclusion is: 3'rd party MDL components can not use Ripple without manually adding the CSS class mdl-js-ripple-effect--ignore-events in markup?

dgrubelic commented 8 years ago

Right! Since MDL internal components are all compiled before ripple component.

vmateixeira commented 8 years ago

@leifoolsen You're welcome! Hope I helped somehow. And I think that is likely to be the case as MaterialRipple is the last component being registered in material.js.

leifoolsen commented 8 years ago

Shouldn't an embedded component initialize after the owner?

dgrubelic commented 8 years ago

Well, this is the lineup for compile:

mdl button
mdl textfield
... (other components)...
mdl ripple
... after some code...
your selectfield

Selectfield should be a part of MDL compile process, meaning it should be a part of MDL components library.

leifoolsen commented 8 years ago

So, we can consider this to be a bug?

dgrubelic commented 8 years ago

Well, no. From MDL lib point of view, i believe that this is not a bug. But, someone from MDL team should evaluate this.

leifoolsen commented 8 years ago

At least it's an undocumentet feature, We can not expect 3'rd party components to be "baked" into MDL.

Garbee commented 8 years ago

It is a limitation of how Ripples were done in 1.x. They were tightly coupled to components and things are done that way to avoid some race conditions that caused faulty animations or failures in component initialization.

leifoolsen commented 8 years ago

@Garbee: I can live with this limitation in v1. Hopefully this will improve in v2 (maybe using promises). At Work we already have one project in development using MDL - and we have two more projects in the pipe line.

Garbee commented 8 years ago

I don't think promises are the solution to this problem in particular. I would like to see is abstract the ripple component into a truly self-standing component that you can use with any node that can have children nodes. But, figuring that one out isn't exactly trivial given how dynamic it would need to be.

leifoolsen commented 8 years ago

What about using an event emitter? Then the ripple could wait for a response before initializing.

Zodiase commented 8 years ago

What I really want is a dependency manager :p Basically I'd like to define that my component is depending on ripple, and when componentHandler scans through the class names and sees ripple and my component, it would upgrade my component first and then ripple.

dgrubelic commented 8 years ago

I think that the idea is to make MDL lighter in v2, not heavier. :smile: If developer can't figure out that his component doesn't work because ripple is being initialized first, i don't know what's the meaning of life. :smile:

Or you're running on a sarcasm line here? :smiley:

Zodiase commented 8 years ago

Issue #1083 I posted sort of is related to this; upgrade order affects the result and will break things.

Zodiase commented 8 years ago

Lighter while not making developers' life harder would be gold.

dgrubelic commented 8 years ago

In order to make developer's life harder, you need to think before developer does and do a lot of things for the developer, which will eventually lead to larger codebase. :smile:

leifoolsen commented 8 years ago

A decorator should wait for the component to be decorated to initialize - not the other way around. If there are other components in MDL behaving this way - this is a serious issue for me.

Zodiase commented 8 years ago

Maybe make decorations like ripple a mixin type of thing that is invoked only by components who need it, not act on its own?

leifoolsen commented 8 years ago

@Zodiase: Yes, this is related to #1083.

leifoolsen commented 8 years ago

Mixins in EcmaScript2016 is simple, just a @mixin decorator. See e.g. https://github.com/jayphelps/core-decorators.js

google / material-design-lite

MDL Ripple component is created/initialized before custom component. #4205