Skip to content
This repository was archived by the owner on May 5, 2021. It is now read-only.

Various Fixes#78

Open
msigley wants to merge 7 commits intoCodeinwp:developfrom
msigley:master
Open

Various Fixes#78
msigley wants to merge 7 commits intoCodeinwp:developfrom
msigley:master

Conversation

@msigley
Copy link
Copy Markdown

@msigley msigley commented Mar 24, 2016

Fixes live NodeList bug in IE. As a side effect the element selection seems to now be more performant.

gilbitron and others added 4 commits February 6, 2016 17:34
…Made the element selection more performant.
…tor fallback. Removed debugging calls. Added support for the Media Query 4 shim for more accurate touch support detection in chrome.
@msigley msigley changed the title Fixes live NodeList bug in IE. Various Fixes Mar 24, 2016
@msigley
Copy link
Copy Markdown
Author

msigley commented Mar 24, 2016

Added Media Query 4 support via this shim:
https://github.com/msigley/mq4-hover-shim

Comment thread ideal-image-slider.js
beforeChange: function() {},
afterChange: function() {}
};
this.length = 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure length is a very good name. What does it do?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It allows one to test for the existence of empty Sliders, sliders with no images. Here is a usage example:

jQuery(document).ready(function ($) {
    var homeSlider = new IdealImageSlider.Slider({
        selector: '#home-slider',
        height: 'auto',
        effect: 'fade',
        interval: 5000,
        transitionDuration: 600,
        maxHeight: 375
    });

    if( homeSlider.length ) {
        homeSlider.addBulletNav();
        homeSlider.start();
    }

Comment thread ideal-image-slider.js
}

//Add loading class
_addClass(sliderEl, 'iis-loading');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you want to add loading classes, they should be in a separate PR.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In the future I'll adhere to a vary feature independent PR style. In the name of closing this PR, can we just keep the loading classes in for now?

Comment thread ideal-image-slider.js
|| !!('ontouchstart' in document.documentElement)
|| !!window.ontouchstart
|| (!!window.Touch && !!window.Touch.length)
|| !!window.onmsgesturechange || (window.DocumentTouch && window.document instanceof window.DocumentTouch)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The spacing for these conditions needs fixed.

Comment thread ideal-image-slider.js

//Remove loading class
_removeClass(sliderEl, 'iis-loading');
_addClass(sliderEl, 'iis-loaded');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above. Loading classes should be in a separate PR.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In the future I'll adhere to a vary feature independent PR style. In the name of closing this PR, can we just keep the loading classes in for now?

Copy link
Copy Markdown
Author

@msigley msigley left a comment

Choose a reason for hiding this comment

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

Made requested code changes and answered code questions.

Comment thread ideal-image-slider.js
}

//Add loading class
_addClass(sliderEl, 'iis-loading');
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In the future I'll adhere to a vary feature independent PR style. In the name of closing this PR, can we just keep the loading classes in for now?

Comment thread ideal-image-slider.js

//Remove loading class
_removeClass(sliderEl, 'iis-loading');
_addClass(sliderEl, 'iis-loaded');
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In the future I'll adhere to a vary feature independent PR style. In the name of closing this PR, can we just keep the loading classes in for now?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants