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.

Sign up for GitHub

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

Bugfix 327 - Responsive Settings via setOption #1334

Closed
wants to merge 1 commit into from
Closed

Bugfix 327 - Responsive Settings via setOption #1334

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 75 additions & 37 deletions slick/slick.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@

function Slick(element, settings) {

var _ = this,
dataSettings, responsiveSettings, breakpoint;
var _ = this, dataSettings;

_.defaults = {
accessibility: true,
Expand Down Expand Up @@ -144,27 +143,6 @@
_.currentSlide = _.options.initialSlide;

_.originalSettings = _.options;
responsiveSettings = _.options.responsive || null;

if (responsiveSettings && responsiveSettings.length > -1) {
_.respondTo = _.options.respondTo || 'window';
for (breakpoint in responsiveSettings) {
if (responsiveSettings.hasOwnProperty(breakpoint)) {
_.breakpoints.push(responsiveSettings[
breakpoint].breakpoint);
_.breakpointSettings[responsiveSettings[
breakpoint].breakpoint] =
responsiveSettings[breakpoint].settings;
}
}
_.breakpoints.sort(function(a, b) {
if (_.options.mobileFirst === true) {
return a - b;
} else {
return b - a;
}
});
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove the breakpoint registration from setup, and place in to it's own method with de-duplication

if (typeof document.mozHidden !== 'undefined') {
_.hidden = 'mozHidden';
Expand Down Expand Up @@ -192,8 +170,9 @@
// Extracted from jQuery v1.11 source
_.htmlExpr = /^(?:\s*(<[\w\W]+>)[^>]*)$/;

_.init(true);

_.registerBreakpoints();
_.init(true);
_.checkResponsive(true);

}
Expand Down Expand Up @@ -572,7 +551,7 @@

};

Slick.prototype.checkResponsive = function(initial) {
Slick.prototype.checkResponsive = function(initial, forceUpdate) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

without forceUpdate the checkResponsive method would never be able to refresh the slider when someone adds breakpoints via setOption() method.


var _ = this,
breakpoint, targetBreakpoint, respondToWidth, triggerBreakpoint = false;
Expand All @@ -587,8 +566,9 @@
respondToWidth = Math.min(windowWidth, sliderWidth);
}

if (_.originalSettings.responsive && _.originalSettings
.responsive.length > -1 && _.originalSettings.responsive !== null) {
if ( _.options.responsive &&
_.options.responsive.length &&
_.options.responsive !== null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was wroing, using _.originalSettings.responsive as now the options can be set post-initilisation so we need to check if responsive was added to _.options


targetBreakpoint = null;

Expand All @@ -608,7 +588,7 @@

if (targetBreakpoint !== null) {
if (_.activeBreakpoint !== null) {
if (targetBreakpoint !== _.activeBreakpoint) {
if (targetBreakpoint !== _.activeBreakpoint || forceUpdate) {
_.activeBreakpoint =
targetBreakpoint;
if (_.breakpointSettings[targetBreakpoint] === 'unslick') {
Expand Down Expand Up @@ -1127,6 +1107,7 @@
if (!$(_.$slider).hasClass('slick-initialized')) {

$(_.$slider).addClass('slick-initialized');

_.buildRows();
_.buildOut();
_.setProps();
Expand All @@ -1135,6 +1116,7 @@
_.initializeEvents();
_.updateArrows();
_.updateDots();

}

if (creation) {
Expand Down Expand Up @@ -1472,6 +1454,46 @@

};

Slick.prototype.registerBreakpoints = function() {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you annotate this? Is it thoroughly tested? Explain what it does and the necessity.


var _ = this, breakpoint, currentBreakpoint, l,
responsiveSettings = _.options.responsive || null;

if ( $.type(responsiveSettings) === "array" && responsiveSettings.length ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just a sanity check to make sure _.options.responsive is actually a usable array and not a retard value.


_.respondTo = _.options.respondTo || 'window';

for ( breakpoint in responsiveSettings ) {

l = _.breakpoints.length-1;
currentBreakpoint = responsiveSettings[breakpoint].breakpoint;

if (responsiveSettings.hasOwnProperty(breakpoint)) {

// loop through the breakpoints and cut out any existing
// ones with the same breakpoint number, we don't want dupes.
while( l >= 0 ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Check each breakpoint inside the _.options.responsive array,
Check that the current breakpoint we're registering (lets say 800) doesn't already exist anywhere in teh _.options.responsive option. -- if it does, splice it out of the array.

Basically, I tried it without this, and noticed that if I supplied an array with a breakpoints which already exist, then it would just take the first one it found in the array when we get to the checkResponsive() method. -- so the decision is that if I'm adding a breakpoint of 800 then, I do not want any current breakpoints which are also at 800 as only the first one would trigger, anyway. Also after setting options later, it was anyone's guess which settings would be triggered due to the sorting algorithm.

if( _.breakpoints[l] && _.breakpoints[l] === currentBreakpoint ) {
_.breakpoints.splice(l,1);
}
l--;
}

_.breakpoints.push(currentBreakpoint);
_.breakpointSettings[currentBreakpoint] = responsiveSettings[breakpoint].settings;

}

}

_.breakpoints.sort(function(a, b) {
return ( _.options.mobileFirst ) ? a-b : b-a;
});

}

};

Slick.prototype.reinit = function() {

var _ = this;
Expand All @@ -1489,22 +1511,19 @@
_.currentSlide = 0;
}

_.setProps();
_.registerBreakpoints();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this allows us to re-register the breakpoints when the setOption() method runs.


_.setProps();
_.setupInfinite();

_.buildArrows();

_.updateArrows();

_.initArrowEvents();

_.buildDots();

_.updateDots();

_.initDotEvents();

_.checkResponsive(false, true);

if (_.options.focusOnSelect === true) {
$(_.$slideTrack).children().on('click.slick', _.selectHandler);
}
Expand Down Expand Up @@ -1680,8 +1699,27 @@

Slick.prototype.setOption = Slick.prototype.slickSetOption = function(option, value, refresh) {

var _ = this;
_.options[option] = value;
var _ = this, l, item;

if( option === "responsive" && $.type(value) === "array" ) {
for ( item in value ) {
if( $.type( _.options.responsive ) !== "array" ) {
_.options.responsive = [ value[item] ];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if originally the _.options.responsive was null then we want to wrap the current object in an array. -- otherwise we want to remove any duplicate breakpoints, and then push the current breakpoint.

} else {
l = _.options.responsive.length-1;
// loop through the responsive object and splice out duplicates.
while( l >= 0 ) {
if( _.options.responsive[l].breakpoint === value[item].breakpoint ) {
_.options.responsive.splice(l,1);
}
l--;
}
_.options.responsive.push( value[item] );
}
}
} else {
_.options[option] = value;
}

if (refresh === true) {
_.unload();
Expand Down