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

Remove 'columns' and 'options' object replacement on initialisation #805

Closed
wants to merge 1 commit into from
Closed

Remove 'columns' and 'options' object replacement on initialisation #805

Show file tree
Hide file tree
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
11 changes: 11 additions & 0 deletions slick.core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,16 @@
callback();
}

function applyDefaults(targetObj, srcObj) {
for (const key in srcObj) {
if (srcObj.hasOwnProperty(key)) {
if (!targetObj.hasOwnProperty(key)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

these nested if could be regrouped under a single if since there's only 1 execution logic underneath

Copy link
Owner Author

Choose a reason for hiding this comment

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

true

targetObj[key] = srcObj[key];
}
}
}
}

// jQuery's extend
var getProto = Object.getPrototypeOf;
var class2type = {};
Expand Down Expand Up @@ -908,6 +918,7 @@
"hide": hide,
"slideUp": slideUp,
"slideDown": slideDown,
"applyDefaults": applyDefaults,
"storage": {
// https://stackoverflow.com/questions/29222027/vanilla-alternative-to-jquery-data-function-any-native-javascript-alternati
_storage: new WeakMap(),
Expand Down
60 changes: 45 additions & 15 deletions slick.grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ if (typeof Slick === "undefined") {
ffMaxSupportedCssHeight: 6000000,
maxSupportedCssHeight: 1000000000,
sanitizer: undefined, // sanitize function, built in basic sanitizer is: Slick.RegexSanitizer(dirtyHtml)
logSanitizedHtml: false // log to console when sanitised - recommend true for testing of dev and production
logSanitizedHtml: false, // log to console when sanitised - recommend true for testing of dev and production
useLegacySettingObjectConfiguration: false // if true, create new object when creating options and columns objects,
// breaking references
};

var columnDefaults = {
Expand Down Expand Up @@ -329,7 +331,12 @@ if (typeof Slick === "undefined") {

// calculate these only once and share between grid instances
maxSupportedCssHeight = maxSupportedCssHeight || getMaxSupportedCssHeight();
options = utils.extend(true, {}, defaults, options);
if (options && options.useLegacySettingObjectConfiguration) {
options = utils.extend(true, {}, defaults, options);
} else {
if (!options) { options = {}; }
Slick.Utils.applyDefaults(options, defaults);
}
validateAndEnforceOptions();
columnDefaults.width = options.defaultColumnWidth;

Expand Down Expand Up @@ -3052,10 +3059,17 @@ if (typeof Slick === "undefined") {
function updateColumnProps() {
columnsById = {};
for (var i = 0; i < columns.length; i++) {
if (columns[i].width) { columns[i].widthRequest = columns[i].width; }
var m = columns[i];
if (m.width) { m.widthRequest = m.width; }

var m = columns[i] = utils.extend({}, columnDefaults, columns[i]);
m.autoSize = utils.extend({}, columnAutosizeDefaults, m.autoSize);
if (options && options.useLegacySettingObjectConfiguration) {
m = columns[i] = utils.extend({}, columnDefaults, columns[i]);
m.autoSize = utils.extend({}, columnAutosizeDefaults, m.autoSize);
} else {
Slick.Utils.applyDefaults(m, columnDefaults);
if (!m.autoSize) { m.autoSize = {}; }
Slick.Utils.applyDefaults(m.autoSize, columnAutosizeDefaults);
}

columnsById[m.id] = i;
if (m.minWidth && m.width < m.minWidth) {
Expand Down Expand Up @@ -3108,30 +3122,44 @@ if (typeof Slick === "undefined") {
return options;
}

function updateOptions(suppressRender, suppressColumnSet, suppressSetOverflow) {
prepareForOptionsChange();
trigger(self.onUpdateOptions, { "options": options });
internal_setOptions(suppressRender, suppressColumnSet, suppressSetOverflow);
}

function setOptions(args, suppressRender, suppressColumnSet, suppressSetOverflow) {
prepareForOptionsChange();

var originalOptions = utils.extend(true, {}, options);
options = utils.extend(options, args);
trigger(self.onSetOptions, { "optionsBefore": originalOptions, "optionsAfter": options });

internal_setOptions(suppressRender, suppressColumnSet, suppressSetOverflow);
}

function prepareForOptionsChange() {
if (!getEditorLock().commitCurrentEdit()) {
return;
}

makeActiveCellNormal();

if (args.showColumnHeader !== undefined) {
setColumnHeaderVisibility(args.showColumnHeader);
}

if (options.enableAddRow !== args.enableAddRow) {
//if (typeof args.enableAddRow !== 'undefined' && options.enableAddRow !== args.enableAddRow) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this code really have to be commented out? isn't that going to break something?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I don't think so, I decided it was actually redundant, but I'd have to double check. We no longer have the before and after state of the options.

invalidateRow(getDataLength());
//}
}

function internal_setOptions(suppressRender, suppressColumnSet, suppressSetOverflow) {
if (options.showColumnHeader !== undefined) {
setColumnHeaderVisibility(options.showColumnHeader);
}

var originalOptions = utils.extend(true, {}, options);
options = utils.extend(options, args);
trigger(self.onSetOptions, { "optionsBefore": originalOptions, "optionsAfter": options });

validateAndEnforceOptions();
setFrozenOptions();

// when user changed frozen row option, we need to force a recalculation of each viewport heights
if (args.frozenBottom !== undefined) {
if (options.frozenBottom !== undefined) {
enforceFrozenRowHeightRecalc = true;
}

Expand Down Expand Up @@ -6205,6 +6233,7 @@ if (typeof Slick === "undefined") {
"onBeforeUpdateColumns": new Slick.Event(),
"onRendered": new Slick.Event(),
"onSetOptions": new Slick.Event(),
"onUpdateOptions": new Slick.Event(),

// Methods
"registerPlugin": registerPlugin,
Expand All @@ -6223,6 +6252,7 @@ if (typeof Slick === "undefined") {
"autosizeColumn": autosizeColumn,
"getOptions": getOptions,
"setOptions": setOptions,
"updateOptions": updateOptions,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this new function seems oddly similar to setOptions, do we really need 2 functions that are similar with similar names? What's the difference between these 2?

Copy link
Owner Author

Choose a reason for hiding this comment

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

covered below

"getData": getData,
"getDataLength": getDataLength,
"getDataItem": getDataItem,
Expand Down