refactor(overlay): leverage "reparentChildren" helper #1290
Conversation
| return function () { | ||
| restoreChildren(placeholderItems, srcElements); | ||
| return function (): Element[] { | ||
| return restoreChildren(placeholderItems, srcElements, slotNames); |
There was a problem hiding this comment.
Use return so that consumers that want to do something more with the elements after their return can.
| 'placement', | ||
| this.originalPlacement | ||
| ); | ||
| element.setAttribute('placement', this.originalPlacement); |
There was a problem hiding this comment.
This is managed here, and slot is managed there... it begs a question should the API be:
reparentChildren(Element[], Element, () => () => void);
Where the double callback at the end amount to prepare and cleanup functionality?
In this way, slot wouldn't have to be managed on everything, and placement or whatever else came up over time, wouldn't have to be cached in the originating code location.
Everything could just be managed/cached at the originating location, just the same, but want to make the workflow clear as one of the follow up tasks to the overall refactor here will be to add this method to: https://opensource.adobe.com/spectrum-web-components/components/shared
There was a problem hiding this comment.
This PR removed the naming of the slot for this element, but the final workflow here would have needed to support the management of the value as well.
There was a problem hiding this comment.
I'm assuming the callbacks would be optional -- definitely makes sense to add more control to the API.
There was a problem hiding this comment.
Check it out in #1310
Westbrook
force-pushed
the
westbrook/overlay-reparenting
branch
from
bb81dd6 to
1a76372
Compare
Westbrook
force-pushed
the
westbrook/overlay-reparenting
branch
6 times, most recently
from
1f67e10 to
1f9e14b
Compare
Westbrook
force-pushed
the
westbrook/overlay-reparenting
branch
from
1f9e14b to
bfda092
Compare
| 'placement', | ||
| this.originalPlacement | ||
| ); | ||
| element.setAttribute('placement', this.originalPlacement); |
There was a problem hiding this comment.
I'm assuming the callbacks would be optional -- definitely makes sense to add more control to the API.
|
|
||
| public menuItems: MenuItem[] = []; | ||
| private restoreChildren?: Function; | ||
| private restoreChildren?: () => void; |
There was a problem hiding this comment.
Ahhh -- still learning typescript here. This makes sense to me.
Westbrook
force-pushed
the
westbrook/overlay-reparenting
branch
from
bfda092 to
3445446
Compare
Description
Use the
reparentChildrenhelper inoverlay-trigger.Related Issue
fixes #1249
Motivation and Context
Simplify code by leveraging helper.
How Has This Been Tested?
CI.
Types of changes
Checklist: