container is not set when code is compressed with webpack about sweet-scroll HOT 28 CLOSED

Also, as a sidenote, you add an Event (addEvent) when the dom hasn't loaded yet, but this doesn't seem to get removed once you reach the callback.call line?

from sweet-scroll.

Hi @nicolechung. Thanks you for issues.
I will confirm that afterward 😃

from sweet-scroll.

It was confirmed by Chrome (52.0.2743.116 (64-bit)) but was not able to reproduce ... 😢

In the following code have been removed the container, but do you have any reason? (Removed when compiled with webpack?)

if (horizontalScroll) {
  container = scrollableFind(selector, "x");
}

You've got a good point. DOMContentLoaded and load event is likely to need to be canceled 👍

Can you send me a sample code including webpack.config.js?

from sweet-scroll.

Here is the webpack config:

base

var path = require('path')

module.exports = {
  entry: {
    app: './src/main.js'
  },
  output: {
    path: path.resolve(__dirname, '../dist/static'),
    publicPath: 'static/',
    filename: '[name].js'
  },
  resolve: {
    extensions: ['', '.js', '.vue'],
    fallback: [path.join(__dirname, '../node_modules')],
    alias: {
      'src': path.resolve(__dirname, '../src'),
      'config': path.join(__dirname, '../config', process.env.NODE_ENV)
    }
  },
  resolveLoader: {
    fallback: [path.join(__dirname, '../node_modules')]
  },
  module: {
    preLoaders: [
      {
        test: /\.vue$/,
        loader: 'eslint',
        exclude: /node_modules/
      },
      {
        test: /\.js$/,
        loader: 'eslint',
        exclude: /node_modules/
      }
    ],
    loaders: [
      {
        test: /\.vue$/,
        loader: 'vue'
      },
      {
        test: /\.js$/,
        loader: 'babel',
        exclude: /node_modules/
      },
      {
        test: /\.json$/,
        loader: 'json'
      },
      {
        test: /\.html$/,
        loader: 'vue-html'
      },
      {
        test: /\.(png|jpg|gif|svg)$/,
        loader: 'url',
        query: {
          limit: 10000,
          name: '[name].[ext]?[hash:7]'
        }
      },
      {
        test: /\.css$/,
        loader: 'style-loader!css-loader'
      },
      {
        test: /\.scss$/,
        loaders: ["style", "css", "sass"]
      }
    ]
  },
  eslint: {
    formatter: require('eslint-friendly-formatter')
  }
}

prod

var webpack = require('webpack')
var config = require('./webpack.base.conf')
var cssLoaders = require('./css-loaders')
var ExtractTextPlugin = require('extract-text-webpack-plugin')
var HtmlWebpackPlugin = require('html-webpack-plugin')

config.bail = true

// naming output files with hashes for better caching.
// dist/index.html will be auto-generated with correct URLs.
config.output.filename = '[name].[chunkhash].js'
config.output.chunkFilename = '[id].[chunkhash].js'

// whether to generate source map for production files.
// disabling this can speed up the build.
var SOURCE_MAP = true

config.devtool = SOURCE_MAP ? '#source-map' : false

config.vue = config.vue || {}
config.vue.loaders = config.vue.loaders || {}
cssLoaders({
  sourceMap: SOURCE_MAP,
  extract: true
}).forEach(function (loader) {
  config.vue.loaders[loader.key] = loader.value
})

config.plugins = (config.plugins || []).concat([
  // http://vuejs.github.io/vue-loader/workflow/production.html
  new webpack.DefinePlugin({
    'process.env': {
      NODE_ENV: '"build"'
    }
  }),
  new webpack.optimize.UglifyJsPlugin({
    compress: {
      warnings: false
    },
    mangle: {
      except: ['SweetScroll']
    }
  }),
  new webpack.optimize.DedupePlugin(),
  new webpack.optimize.OccurenceOrderPlugin(),
  // extract css into its own file
  new ExtractTextPlugin('[name].[contenthash].css'),
  // generate dist index.html with correct asset hash for caching.
  // you can customize output by editing /index.html
  // see https://github.com/ampedandwired/html-webpack-plugin
  new HtmlWebpackPlugin({
    filename: '../index.html',
    template: 'index.html',
    inject: true,
    minify: {
      removeComments: true,
      collapseWhitespace: true,
      removeAttributeQuotes: true
      // more options:
      // https://github.com/kangax/html-minifier#options-quick-reference
    }
  })
])

module.exports = config

from sweet-scroll.

this is the result code after webpack (looks like horizontalScroll is not removed for me?)

{
          key: "getContainer",
          value: function getContainer(selector, callback) {
            var _this3 = this;

            var _options = this.options;
            var verticalScroll = _options.verticalScroll;
            var horizontalScroll = _options.horizontalScroll;

            var container = null;

            if (verticalScroll) {
              container = scrollableFind(selector, "y");
            }

            if (!container && horizontalScroll) {
              container = scrollableFind(selector, "x");
            }

                // note: if container is null HERE but Dom is loaded, it goes to the callback!
            if (!container && !isDomContentLoaded) {
              (function () {
                var isCompleted = false;

                addEvent(doc, DOM_CONTENT_LOADED, function () {
                  isCompleted = true;
                  _this3.getContainer(selector, callback);
                });

                // Fallback for DOMContentLoaded
                addEvent(win, "load", function () {
                  if (!isCompleted) {
                    _this3.getContainer(selector, callback);
                  }
                });
              })();
            } else {
              callback.call(this, container);
            }
          }

Like, without being to recreate the DOM issue (you need to have a big webpack with lots of JS to recreate) you can see that it's possible for the container to be null AND the dom to be loaded. If so, because the conditional uses && instead of || it's possible for the callback to be called with null container (if that makes sense).

I'm not entirely sure how scrollableFind works (I only scanned the code) but I think it's possible for those functions to not work (because the Dom is still loading), so container is still null, but then by the time you reach the last conditional, the dom might be loaded by then.

from sweet-scroll.

Thank you for webpack.config.js.

That makes sense. I think the following code as improvement. (It is the previous code to be compiled)

getContainer(selector, callback) {
  const { verticalScroll, horizontalScroll } = this.options;
  const finalCallback = callback.bind(this);
  let container = null;

  if (verticalScroll) {
    container = Dom.scrollableFind(selector, "y");
  }

  if (!container && horizontalScroll) {
    container = Dom.scrollableFind(selector, "x");
  }

  // Note: End if find a container.
  if (container) {
    finalCallback(container);

  // Note: If container not found, and DOM has not been built, to find the container.
  } else if (!isDomContentLoaded) {
    let isCompleted = false;

    const handleDomContentLoaded = () => {
      isCompleted = true;
      removeEvent(doc, DOM_CONTENT_LOADED, handleDomContentLoaded);
      this.getContainer(selector, callback);
    };

    const handleLoad = () => {
      removeEvent(win, LOAD, handleLoad);
      if (!isCompleted) {
        this.getContainer(selector, callback);
      }
    };

    addEvent(doc, DOM_CONTENT_LOADED, handleDomContentLoaded);
    addEvent(win, LOAD, handleLoad);

  } else {
    finalCallback(null);
  }
}

This seems to solve the previous problem. (I tried this code apply to the demo page. It is operating normally)

What do you think?

from sweet-scroll.

Let me test it, I will get back to you in two hours? Thank you for the quick response!

from sweet-scroll.

You're welcome!
It's getting late over here. I'll try to check tomorrow :)

from sweet-scroll.

I tested and this looks good to me.

Can you let me know if / when the Sweet Scroll library is updated with the above ^^ change?

Thanks so much for looking into this!

from sweet-scroll.

Thanks for test. And I intend to update sweet-scroll.js.
I will let you know after I released it. Please wait a little 😃

Thank you, too!

from sweet-scroll.

A bug fix version has been released.
Please try it if you have the chance :)

sweet-scroll - npm

from sweet-scroll.

Hi, you are (I am guessing) sleeping, but since you will be awake when I am sleeping I will leave a note.

I tested this again and pushed it out to another environment, and it did not work (don't ask...it worked in the earlier environment I tested).

But I did notice this line:

else {
        finalCallback(null);
      }```

The problem with that is that it `finalCallback` (mostly) refers to this function:

```JS
function (target) {
      _this.container = target;
      _this.header = $(params.header);
      _this.tween = new ScrollTween(target);
      _this._trigger = null;
      _this._shouldCallCancelScroll = false;
      _this.bindContainerClick();
      _this.hook(params, "initialized");
    }```

If you say `finalCallback(null)` then the `target` will be null, which means the `container` will be set to null. Which means when I try to scroll using `toElement` it won't work (since the container is null).

Hope this makes sense and if you have time to take a look at in in the next few weeks that would be a super awesome help.

from sweet-scroll.

One question.
Do you specify a value for the second argument of SweetScroll constructor?
I would be happy if you know anything, please tell ;-)

I want to pursue the cause of the scrollable element not found bug.
thanks!

from sweet-scroll.

We tried the following (all different scenarios)

let sweetScroll = new SweetScroll({}, 'body')

let sweetScroll = new SweetScroll({}, document.querySelector('body'))

let sweetScroll = new SweetScroll({})

In each case, the SweetScroll always works when we don't minify our code. But then when we minify it breaks.

Now it works when I minify but it breaks when we move it to our staging environment. I think because it loads at a different speed again (some async condition between the dom being ready and the container getting initialized)

Again, thank you for taking the time to look at this.

from sweet-scroll.

My apologies for the late reply.
When do you execute they codes?

• Immediately after loading of script
• After DOMContentLoaded
• After onload
• Other...

Thank you for doing...so many times!

from sweet-scroll.

I'm using vuejs, it installs as a "vue plugin"...so I think before the DOMContentLoads.

I tried putting it in a domcontentload listener, but then I noticed that the sweet scroll plugin already listens for domloading?

from sweet-scroll.

Oh, that makes sense. sweet-scroll.js is listened for DOMContentLoaded.
Would you debug to scrollableFind processing?

PS: In addition, whether it would be better to create a plug-in of Vue.js if necessary?

from sweet-scroll.

scrollables for "y" is an empty array (length 0) for "selectors: body".

That happens twice.

For direction "x" that does not occur.

from sweet-scroll.

But also, when you use toElement if the container is null, then the scrolling won't work. If you have the final callback with null as a parameter, when can the container not be null after that?

from sweet-scroll.

For example, will you work the following code? (getContainer() method)

  getContainer(selector, callback) {
    const { verticalScroll, horizontalScroll } = this.options;
    const finalCallback = callback.bind(this);
    let container = null;

    if (verticalScroll) {
      container = Dom.scrollableFind(selector, "y");
    }

    if (!container && horizontalScroll) {
      container = Dom.scrollableFind(selector, "x");
    }

    if (container) {
      finalCallback(container);

    // Note: Check readyState
    } else if (!/comp|inter|loaded/.test(doc.readyState)) {
      let isCompleted = false;

      const handleDomContentLoaded = () => {
        removeHandlers();
        isCompleted = true;
        this.getContainer(selector, callback);
      };

      const handleLoad = () => {
        removeHandlers();
        if (!isCompleted) {
          this.getContainer(selector, callback);
        }
      };

      const removeHandlers = () => {
        removeEvent(doc, DOM_CONTENT_LOADED, handleDomContentLoaded);
        removeEvent(win, LOAD, handleLoad);
      };

      addEvent(doc, DOM_CONTENT_LOADED, handleDomContentLoaded);
      addEvent(win, LOAD, handleLoad);

    } else {
      setTimeout(() => this.getContainer(selector, callback), 1);
    }
  }

Do not listen of DOMContentLoaded in the sweet-scroll.js. It has been changed so as to determine the loading status of the DOM.

from sweet-scroll.

Do you have a built (ES5) version of that code I can drop in and test?

I tried to drop this into the src but I could not build (I think I am
missing some dependencies)

On 21 August 2016 at 23:43, tsuyoshi wada [email protected] wrote:

For example, will you work the following code? (getContainer() method)

getContainer(selector, callback) {
const { verticalScroll, horizontalScroll } = this.options;
const finalCallback = callback.bind(this);
let container = null;

if (verticalScroll) {
  container = Dom.scrollableFind(selector, "y");
}

if (!container && horizontalScroll) {
  container = Dom.scrollableFind(selector, "x");
}

if (container) {
  finalCallback(container);

// Note: Check readyState
} else if (!/comp|inter|loaded/.test(doc.readyState)) {
  let isCompleted = false;

  const handleDomContentLoaded = () => {
    removeHandlers();
    isCompleted = true;
    this.getContainer(selector, callback);
  };

  const handleLoad = () => {
    removeHandlers();
    if (!isCompleted) {
      this.getContainer(selector, callback);
    }
  };

  const removeHandlers = () => {
    removeEvent(doc, DOM_CONTENT_LOADED, handleDomContentLoaded);
    removeEvent(win, LOAD, handleLoad);
  };

  addEvent(doc, DOM_CONTENT_LOADED, handleDomContentLoaded);
  addEvent(win, LOAD, handleLoad);

} else {
  setTimeout(() => this.getContainer(selector, callback), 1);
}

}

Do not listen of DOMContentLoaded in the sweet-scroll.js. It has been
changed so as to determine the loading status of the DOM.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tsuyoshiwada/sweet-scroll/issues/27#issuecomment-241308899,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAvEYl8eRkQUNZQzoJOi6Zw1IUG3XBYBks5qiRrOgaJpZM4JlboO
.

from sweet-scroll.

Sorry about that...
I pushed the code in a different branch. Please try to checkout it.

tsuyoshiwada/sweet-scroll at try-fix-issue-#27

from sweet-scroll.

This appears to work (locally). I tried setting the throlling on my network tab too.

Can you push it to master? Unfortunately (unless you know how) I can't 100% test because our staging environment builds from it's own package.json, so I think it needs to be in master. Unless you know how to make a package.json pull from a (non-master) branch instead??

from sweet-scroll.

Thanks for tests!
But, Can't still merge to master. Because It's to the failure to tests in mocha.

I think the npm doc would be helpful.

from sweet-scroll.

Got it...but so, should I wait for the mocha tests to pass first?

On 22 August 2016 at 09:30, tsuyoshi wada [email protected] wrote:

Thanks for tests!
But, Can't still merge to master. Because It's to the failure to tests in
mocha.

I think the npm doc
https://docs.npmjs.com/files/package.json#git-urls-as-dependencies
would be helpful.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tsuyoshiwada/sweet-scroll/issues/27#issuecomment-241412663,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAvEYodfmN-J_EE0gXLAdHXwJYnJEH6iks5qiaRngaJpZM4JlboO
.

from sweet-scroll.

No, I think that priority test the work on staging environment. (Sorry about my poor English...)

from sweet-scroll.

So I tried

"dependencies": {
"sweet-scroll": "^1.0.4#21bd8a434f",

But I get this error:

npm ERR! No compatible version found: sweet-scroll@^1.0.4#21bd8a434f

npm ERR! Valid install targets:

On 22 August 2016 at 10:36, tsuyoshi wada [email protected] wrote:

No, I think that priority test the work on staging environment. (Sorry
about my poor English...)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tsuyoshiwada/sweet-scroll/issues/27#issuecomment-241433181,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAvEYlSeDr8vdAwC8dPghvAp10Pg1528ks5qibQCgaJpZM4JlboO
.

from sweet-scroll.

Wait, sorry, got it working with:

"sweet-scroll": "git+https://github.com/tsuyoshiwada/sweet-scroll#21bd8a434f
",

Testing now.

On 22 August 2016 at 10:51, Nicole Chung [email protected] wrote:

So I tried

"dependencies": {
"sweet-scroll": "^1.0.4#21bd8a434f",

But I get this error:

npm ERR! No compatible version found: sweet-scroll@^1.0.4#21bd8a434f

npm ERR! Valid install targets:

On 22 August 2016 at 10:36, tsuyoshi wada [email protected]
wrote:

No, I think that priority test the work on staging environment. (Sorry
about my poor English...)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tsuyoshiwada/sweet-scroll/issues/27#issuecomment-241433181,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAvEYlSeDr8vdAwC8dPghvAp10Pg1528ks5qibQCgaJpZM4JlboO
.

from sweet-scroll.