fix(value-list, sortable-list): fix nested sorting components #6983
Conversation
driskull
changed the title
github-actions
bot
added
the
bug
…nents into dris0000/sortable-fix
| this.cleanUpDragAndDrop(); | ||
| this.items = Array.from(this.el.children); | ||
| this.setUpDragAndDrop(); | ||
| this.setUpSorting(); |
There was a problem hiding this comment.
setUpSorting handles the preceding logic already.
| this.items = Array.from(this.el.children); | ||
| this.setUpDragAndDrop(); | ||
| connectSortableComponent(this); | ||
| this.setUpSorting(); |
There was a problem hiding this comment.
setUpSorting handles the preceding logic already.
| @@ -166,15 +173,16 @@ export class SortableList implements InteractiveComponent { | |||
| this.items = Array.from(this.el.children); | |||
|
|
|||
| handle.activated = true; | |||
| handle.setFocus(); | |||
| focusElement(handle); | |||
There was a problem hiding this comment.
calls setFocus if applicable to the handle.
| this.mutationObserver?.disconnect(); | ||
| this.cleanUpDragAndDrop(); |
There was a problem hiding this comment.
disconnectSortableComponent handles this already.
| @@ -220,7 +235,6 @@ export class ValueList< | |||
|
|
|||
| componentDidLoad(): void { | |||
| setComponentLoaded(this); | |||
| this.setUpDragAndDrop(); | |||
There was a problem hiding this comment.
This is better handled in connectedCallback since componentDidLoad is only called once. If a component is removed then the sortable will be destroyed in disconnectedCallback so this is needed here.
| requestAnimationFrame(() => focusElement(handle)); | ||
|
|
||
| if ("activated" in handle) { | ||
| handle.activated = true; |
There was a problem hiding this comment.
since handleSelector can be anything we need to do this check. Maybe we shouldn't support a handleSelector if we want to maintain drag consistency thoughout components and require use of a calcite-handle.
|
|
||
| const handle = event | ||
| .composedPath() | ||
| .find((el: HTMLElement) => el.matches(this.handleSelector)) as HTMLElement; |
There was a problem hiding this comment.
Need to find the handle in the composedPath. This was broken.
| @@ -313,31 +327,34 @@ export class ValueList< | |||
| this.filteredItems = filteredItems; | |||
| }; | |||
|
|
|||
| setUpDragAndDrop(): void { | |||
| this.cleanUpDragAndDrop(); | |||
| setUpSorting(): void { | |||
There was a problem hiding this comment.
Would this be something we need defined in the interface as well?
There was a problem hiding this comment.
It could be but it doesn't need to be. I left it out for now since it wouldn't do anything by being added to the interface.
driskull
added
the
pr ready for visual snapshots
Related Issue: #6024
Summary
The main problem is:
Sortableinstance onconnectedCallbackand destroySortableondisconnectedCallback.Sortablecomponent with anotherSortablecomponent inside of it, it triggers the child component'sconnectedCallback/disconnectedCallbackmethods which initialize/destroy aSortable. This causes JS errors and UI problems.connectedCallback/disconnectedCallbackfrom doing anything with the inactiveSortableinstances. (not destroy them & recreate them).Solution:
SortableComponentcomponents to follow.