Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Universal module definition (umd) support added #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

imefisto
Copy link

Hi ! I've used your code with requirejs, so I've added UMD support.

Thank you.

Copy link
Owner

@2is10 2is10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Could you do this without touching so many lines? With the factory function added, the IIFE is no longer needed. Seems like the file could start with this:

// github.com/2is10/selectionchange-polyfill

(function (root, factory) {
    if (typeof define === "function" && define.amd) {
        define([], factory);
    } else if (typeof exports === "object") {
        module.exports = factory();
    } else {
        root.selectionchange = factory();
    }
}(this, function (undefined) {
  var MAC = /^Mac/.test(navigator.platform);
  ...

...and nothing else would need to change except removing this from the bottom:

if (typeof module !== 'undefined') {
  // CommonJS/Node compatibility.
  module.exports = selectionchange;
}

@2is10
Copy link
Owner

2is10 commented Feb 14, 2017

Also, please stick with the indentation level this file is already using: 2 spaces. Thanks.

P.S. Good news, Firefox 52 (scheduled for release March 7) has the selection events API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants