fixes in updateSelector() #142
Conversation
|
I didn't change the deleteSelector method because it works correctly and deletes the parent selector and its childs as it should be. In my opinion, the problem here in the updateSelector method, which uses deleteSelector, although in theory the selector may not be deleted, but an existing one can be changed. |
|
Also, I must say that my solution has a bug. It keeps the child selectors, but the grandchildren are removed(children's children). I couldn't find an efficient way to cascade through all child selectors. So I think I need some help about this |
|
I'm also not sure about VAR, but I think that because it has functional variable scope, and is therefore only accessible from the updateSelectror method, this should not cause any major problems |
|
What happens if you encounter a selector that is a parent of itself? |
| @@ -155,10 +155,20 @@ export default class Sitemap { | |||
| updateSelector(selector, selectorData) { | |||
| // selector is undefined when creating a new one and delete old one, if it exist | |||
| if (selector === undefined || selector.type !== selectorData.type) { | |||
| var CanHaveChilds = selector.canHaveChildSelectors() | |||
There was a problem hiding this comment.
Convert the first letter to lowercase to match the camelCase
And use let or const instead of var (https://skillbox.ru/media/code/chem_razlichayutsya_var_let_i_const_v_javascript/)
| @@ -155,10 +155,20 @@ export default class Sitemap { | |||
| updateSelector(selector, selectorData) { | |||
| // selector is undefined when creating a new one and delete old one, if it exist | |||
| if (selector === undefined || selector.type !== selectorData.type) { | |||
| var CanHaveChilds = selector.canHaveChildSelectors() | |||
| if (selector) { | |||
There was a problem hiding this comment.
I would suggest change this block of code (to the line 172) and move some of its parts into a separate method that will receive the ID of the old and new selectors as input, and then replace the ID of the old selector with the ID of the new selector in all direct child selectors
Previously, when changing a selector type, all of its childrens were deleted, even if the new selector type supported child selectors. This change fixes it by creating a copy of the child selectors and adding them as children to the new parent. (TALCR-538)