Enhance MDL for embedded widgets usage

[genadis]

Hi,

First of all thank you for a great product.

Overview

I'm working on an embed-able javascript widget: A one liner javascript to be included by third party websites that provides some service.

I've cloned the MDL repository and done partial review of the code structure.

I'm relatively new to front end web development, but understand that the current implementation intended for website development only and not suited for embedded widgets.

Motivation

I believe, it is not very complicated to make it suited for embedded widgets as well. It would be a great tool for widgets as it's light weight, doesn't have dependencies and just great.

I wanted to share my thoughts and tests I've done. Please tell me what you think, If I'm missing something and whether you think it would be good for the MDL project.

Why I believe TODAY MDL is not suited for embedded widgets

Some of the basic best practices to not interfere with the website and not let it interfere with the widget are:

1. Keep the global scope clear from javascript code (objects, functions, variables)

MDL sets all the components and componentHandler on the global window object

2. Do not interfere with CSS: encapsulate all the css in widgets container element

Although MDL has mdl-* prefix for all of it's classes, all the classes applied globally and there is no way to encapsulate it.

3. Load everything asynchronously to not interfere with the website loading times

It is not an issue but, would be great if MDL was encapsulated and exported as ECMAScript 6 module (I believe there are already opened issues on that topic).

Suggestions

Encapsulate the MDL Javascript

1. Encapsulate components in single mdl object.
2. Allow encapsulating mdl object in vendor scope.
3. Or As alternative to option 2 - export as module.

I've done a small patch to see if encapsulating doesn't break MDL. Seems it doesn't as the JS Objects applied directly on the relevant DOM elements by the upgradeAllRegistered().

This is my patch (not suited for final solution):

Added wrapPatch.js to gulp SOURCES

const SOURCES = [
  'src/wrapPatch.js',
  // Component handler
  'src/mdlComponentHandler.js',
  /* ... */
} 

and excluded it from lint.

gulp.task('lint', () => {
  return gulp.src([
      'src/**/*.js',
      'gulpfile.babel.js',
      '!src/wrapPatch.js'
    ])
    /* ... */
}

Added args and params to iife

gulp.task('scripts', ['lint'], () => {
  return gulp.src(SOURCES)
    /* ... */
    .pipe($.iife({useStrict: true, args: ['window','"MaVendor"'], params: ['gwindow','scope','window']}))
    /* ... */
}

And finally the wrapPatch.js:

  if (false === (gwindow === window)) {
    if ('undefined' === typeof window) {
      window = {};
    }
    window.setTimeout = gwindow.setTimeout.bind(gwindow);
    window.addEventListener = gwindow.addEventListener.bind(gwindow);
    window.removeEventListener = gwindow.removeEventListener.bind(gwindow);
    window.navigator = gwindow.navigator;
    window.matchMedia = gwindow.matchMedia.bind(gwindow);
    //window.requestAnimationFrame = gwindow.requestAnimationFrame;
    //window.cancelAnimationFrame = gwindow.cancelAnimationFrame;
    gwindow[scope] = gwindow[scope] || {};
    gwindow[scope].mdl = window;
  }

I had to disable the mocha tests as well because they expect all the MDL components to be global.

NOTE: The mocha test are easily fixable, for example the first componentHandler Unit test it fixed by adding 2 lines at describe:

  var componentHandler = window["MaVendor"].mdl.componentHandler;
  var MaterialButton = window["MaVendor"].mdl.MaterialButton;

For true solution instead of hard coded "MaVendor", the scope variable from the scripts gulp task should be injected.

This is the result:

As you can see all the MDL related objects are under: window.MaVendor.mdl.
MaterialComponentsNav and MaterialComponentsSnippets are related to the mdl website docs and not part of the release.

Encapsulate/Prefix the MDL CSS

I haven't review the CSS topic enough (wanted to post the issue first), but here are some thoughts:

Note: The thoughts below intended to give options for developers that build the library, not for the end users. It can be done by defining some variable that if the developer chooses to, will be able to add the prefix/encapsulating-class, instead of editing all the components manually.

For maximum flexibility of the end user maybe it is a good idea to expose it to the Custom CSS theme builder tool.

The resulting CSS should look something like this:

.encapsulating-class .mdl-button {
 /* .... */
}

Or

.encapsulating-class .prefix-mdl-button {
 /* .... */
}

The code is well organized and Sass is used for the CSS generation, so adding the encapsulation should not be an issue. All the imports are done by material-design-lite.scss

Add alternative resets suited for embedded widgets

Instead of the used resets maybe some combination with cleanslate can be used.

!important is used across the MDL implementation so it is well suited for embedded widgets.

[Garbee]

Sounds like something to look into for V2.

I had to disable the mocha tests as well because they expect all the MDL components to be global.

Instantly means we can't do anything in V1 due to this being such a major BC break.

---

Encapsulate/Prefix the MDL CSS

On this note, I'd urge we not do this. BEM usage and having the mdl prefix already keeps us fairly clear of clashes. Doing an encapsulation class also increases specificity even more which is a downside to developers trying to modify things. Also adding prefix before the mdl namespace on classes makes little sense overall. It is just going to make an even more confusing classname convention for developers to write out.

[genadis]

Garbee, Thank you for your quick response.

I have updated the issue.

1. mocha tests are not broken, please see the update explaining the simple fix.
2. regarding Encapsulate/Prefix the MDL CSS: updated the chapter as well to explain more clearly the intent.

The mdl prefix does not keep from clashes for embedded widgets, imaging the widget embedded uses MDL version 2.0.0 and the website uses MDL version 1.0.5 with it's custom colors and version.

[Garbee]

Just by the virtue that the tests need to be modified in that way means it is a breaking fix. Since existing sites expect the components to be in global scope. Changing the tests to pass during a breaking change doesn't mean the change doesn't break things in existence any less.

As far as the CSS parent class, I'm not sure why we should be the ones to maintain that. If developers need it, they can easily add it in themselves and generate from SCSS directly.

[genadis]

Sorry for being so persistent.
But what if in tests:

  if('undefined' === typeof componentHandler) {
    var componentHandler = window["MaVendor"].mdl.componentHandler;
    var MaterialButton = window["MaVendor"].mdl.MaterialButton;
  }

Note: just tried it for all of the mocha tests, all passed and gulp all works.

and in gulp by default:

gulp.task('scripts', ['lint'], () => {
  return gulp.src(SOURCES)
    /* ... */
    .pipe($.iife({useStrict: true, args: ['window','"MaVendor", 'window''], params: ['gwindow','scope','window']}))
    /* ... */
}

instead of:

    .pipe($.iife({useStrict: true, args: ['window','"MaVendor"'], params: ['gwindow','scope','window']}))

will cause by default global mdl just like before.
But will allow developers who need it to compile wrapped mdl library.

I guess the why should we question comes up again. For the long run encapsulating I believe would be the better way to go.
Not only for developers that choose to compile the library but for user using the customize builder. Specifying scope - generates wrapped mdl library effecting the JS and CSS.

I'll be happy to contribute by the way.

[genadis]

I've created a fork and implemented clean patch for encapsulation.
Basically it lets you run:

gulp all:encap --vendor 'foo'

All the objects get encapsulated in window.foo.mdl
All tests work.
Nothing is broken when running the usual gulp tasks, global scope is used by default.

It required few small additions to gulpfile.babel.js and test/index.html.
and creation of src/encapsulationPatch,js

I'll work on CSS encapsulation and update.

[GordonSmith]

This gets my vote as well. The fact that MDL touches any global styles (h3, ul, etc.) makes it unusable by any of the "load on demand" libraries (requirejs, systemjs etc.) as loading the CSS will adversely affect the hosting page.

My naive proposal would be to prepend ".mdl" to all the global styles and then let the user add a class='mdl' to their <body> element. (v2?)

[genadis]

MDL is built with SASS, so instead of hard coding the mdl- prefixes and the global .mdl style it would be even better to define those as variables as part of the build process.

In my fork, I've done patch that does "replace", seems to work for me, but it is patch solution.

gulp widget -v foo -p fa`

will build:

.mdl-button {}
/* becomes */
.foo .fa-button {}

Would be great to see such native support.

[GordonSmith]

@genadis My initial concern is actually with the "global" css changes (like to H1, H2, H3 etc.)

[Garbee]

We tried a body class before and it made for a super-wonky setup to modify things. In V2 we are planning on dropping any global touches and simply scoping things within the components only.

[GordonSmith]

@Garbee Thats great to hear! My setup seems to work fine otherwise (with requirejs injecting the js + css (currently my modified version) as needed: http://rawgit.com/GordonSmith/Visualization/MDL/demos/dermatology.html?src/mdl/Surface

(For this to be usable in my scenario I would need some control of the "fixed header" height - which is fine for a page header, but not for inner DIV headers, just too tall...).

[genadis]

@GordonSmith This was one of my concerns as well, that's why the patch adds .mdl (or following my previous example .foo class to globals as well:

h1 {} -> .foo h1 {}
html {} -> .foo-html {}
body {} -> .foo-body {}

more details in the readme of the fork

@Garbee Could you please elaborate on the planned scoping for v2? Thanks!