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.

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

fix(TreeData): Reset the childrens prop when unflattening dataset in case it is being reused #1675

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 7 additions & 7 deletions packages/common/src/core/slickCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ export class SlickRange {
else {
return `(${this.fromRow}:${this.fromCell} - ${this.toRow}:${this.toCell})`;
}
};
}
}


Expand Down Expand Up @@ -470,7 +470,7 @@ export class SlickGroup extends SlickNonDataItem {
this.count === group.count &&
this.collapsed === group.collapsed &&
this.title === group.title;
};
}
}

/**
Expand Down Expand Up @@ -525,7 +525,7 @@ export class SlickEditorLock {
*/
isActive(editController?: EditController): boolean {
return (editController ? this.activeEditController === editController : this.activeEditController !== null);
};
}

/**
* Sets the specified edit controller as the active edit controller (acquire edit lock).
Expand All @@ -547,7 +547,7 @@ export class SlickEditorLock {
throw new Error('SlickEditorLock.activate: editController must implement .cancelCurrentEdit()');
}
this.activeEditController = editController;
};
}

/**
* Unsets the specified edit controller as the active edit controller (release edit lock).
Expand All @@ -563,7 +563,7 @@ export class SlickEditorLock {
throw new Error('SlickEditorLock.deactivate: specified editController is not the currently active one');
}
this.activeEditController = null;
};
}

/**
* Attempts to commit the current edit by calling "commitCurrentEdit" method on the active edit
Expand All @@ -575,7 +575,7 @@ export class SlickEditorLock {
*/
commitCurrentEdit(): boolean {
return (this.activeEditController ? this.activeEditController.commitCurrentEdit() : true);
};
}

/**
* Attempts to cancel the current edit by calling "cancelCurrentEdit" method on the active edit
Expand All @@ -586,7 +586,7 @@ export class SlickEditorLock {
*/
cancelCurrentEdit(): boolean {
return (this.activeEditController ? this.activeEditController.cancelCurrentEdit() : true);
};
}
}

export class Utils {
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/services/dateUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ export function tryParseDate(inputDate?: string | Date, inputFormat?: string, st
export function toUtcDate(inputDate: string | Date): Date {
// to parse as UTC in Tempo, we need to remove the offset (which is a simple inversed offset to cancel itself)
return removeOffset(inputDate, offset(inputDate, 'utc'));
};
}

/**
* Parse a date passed as a string (Date only, without time) and return a TZ Date (without milliseconds)
Expand Down
13 changes: 5 additions & 8 deletions packages/common/src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,10 @@ export function unflattenParentChildArrayToTree<P, T extends P & { [childrenProp
// make them accessible by guid on this map
const all: any = {};

inputArray.forEach((item: any) => all[item[identifierPropName]] = item);
inputArray.forEach((item: any) => {
all[item[identifierPropName]] = item;
delete item[childrenPropName];
Copy link
Owner

@ghiscoding ghiscoding Sep 12, 2024

Choose a reason for hiding this comment

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

the only thing I wonder is why are you deleting after and not before the assignment, so why not inverse lines 197 & 198 ? is it because the item[childrenPropName] can contain children and you want to delete them from the item?

Copy link
Author

Choose a reason for hiding this comment

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

These lines are interchangeable without any affect on each other.

Line 197 maps the item reference under the set id field.
Line 198 removes the children property from the item.

I've added the line here as it was already a place where all items were iterated already.

Copy link
Owner

Choose a reason for hiding this comment

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

ok thanks, let's merge and hope this fixes it all :)

});

// connect childrens to its parent, and split roots apart
Object.keys(all).forEach((id) => {
Expand All @@ -205,13 +208,7 @@ export function unflattenParentChildArrayToTree<P, T extends P & { [childrenProp
if (!(childrenPropName in p)) {
p[childrenPropName] = [];
}
const existIdx = p[childrenPropName]?.findIndex((x: any) => x[identifierPropName] === item[identifierPropName]);
if (existIdx >= 0) {
// replace existing one when already exists (probably equal to the same item in the end)
p[childrenPropName][existIdx] = item;
} else {
p[childrenPropName].push(item);
}
p[childrenPropName].push(item);
if (p[collapsedPropName] === undefined) {
p[collapsedPropName] = options?.initiallyCollapsed ?? false;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/nodeExtend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,4 @@ export function extend<T = any>(...args: any[]): T {

// Return the modified object
return target;
};
}
Loading