Skip to content

[Nested Lists] ListType: reset list when no value is submitted, cast objects with nu… #3776

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

Open
wants to merge 2 commits into
base: nested-lists
Choose a base branch
from
Open

[Nested Lists] ListType: reset list when no value is submitted, cast objects with nu… #3776

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7587825
ListType: reset list when no value is submitted, cast objects with nu…
naptowncode Dec 9, 2016
da69967
ListType: before casting an object to array, check to see if it has '…
naptowncode Dec 9, 2016
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
18 changes: 10 additions & 8 deletions fields/types/list/ListType.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,17 +185,19 @@ list.prototype.updateItem = function (item, data, files, callback) {

var field = this;
var values = this.getValueFromData(data);
// Don't update the value when it is undefined
if (values === undefined) {
return utils.defer(callback);
}
// Reset the value when null or an empty string is provided
if (values === null || values === '') {
// Reset the value when no value or null or an empty string is provided
if (values === undefined || values === null || values === '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a definite change in semantics, I think it would be better to leave the undefined case as-is…

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I was cheating here to fix two bugs at once.

With the undefined case as it is there is no way to empty out the list. The last item will never be deleted because an empty list won't be submitted at all. TextArray, DateArray, and NumberArray all treat undefined the way I've done here.

values = [];
}
// Wrap non-array values in an array
if (!Array.isArray(values)) {
values = [values];
if (typeof values === 'object' && values[0] !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should use hasOwnProperty in this instance, although for this particular use case, I suppose array values should always be at least null? Not sure.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, hasOwnProperty is probably more correct. Thanks!

// Cast an object with numeric keys to an array
values.length = Object.getOwnPropertyNames(values).length;
values = Array.prototype.slice.call(values);
} else {
// Wrap non-array values in an array
values = [values];
}
}
// NOTE - this method will overwrite the entire array, which is less specific
// than it could be. Concurrent saves could lead to race conditions, but we
Expand Down