fix(overlay): returning only the first element for slot assignedElements

[zolar]

Only return first element form assignedElements instead of an array of elements.

Description

This PR reverses a helper method that was used in the previous version of the OverLayTrigger. In that version, the slotchange handler uses assignedNodes instead of assignedElements. I think using assignedElements renders the slot so fast, that our dialog component has not completed loading, leading to forcing a reload, in which it does goes threw the same process.

spectrum-web-components/packages/overlay/src/OverlayTrigger.ts

Line 454 in a532ff8

private extractSlotContentFromEvent(event: Event): HTMLElement | undefined {

Related issue(s)

[Bug]: OverlayTrigger can get stuck in a render loop causing page to crash

Motivation and context

Resolves an issue related to endless render loop.

How has this been tested?

• Did it pass in Desktop?
• Did it pass in Mobile?
• Did it pass in iPad?

Screenshots (if appropriate)

N/A

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

• I have signed the Adobe Open Source CLA.
• My code follows the code style of this project.
• If my change required a change to the documentation, I have updated the documentation in this pull request.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

[Rajdeepc]

We would like to see you use case here considering this is very specific. Can you add a video or a repro to show how does this solve your issue?

[zolar]

We would like to see you use case here considering this is very specific. Can you add a video or a repro to show how does this solve your issue?

@Rajdeepc Yes, I can add a video or we can do another debug session so I can walk threw it with you or you team?

[zolar]

@Rajdeepc This is a video of the component not rendering. The component container stays blank and you can see the console output going forever. It will eventually crash the browser.

Sequence.low.02.mp4

[Rajdeepc]

@Rajdeepc This is a video of the component not rendering. The component container stays blank and you can see the console output going forever. It will eventually crash the browser.

Sequence.low.02.mp4

This seems to be a condition where the entire component is re-rendering multiple times due to state or property changes. Can you do a repro of your use case by any means?

[changeset-bot]

⚠️ No Changeset found

Latest commit: 38d3f0e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[zolar]

@Rajdeepc This is a video of the component not rendering. The component container stays blank and you can see the console output going forever. It will eventually crash the browser.
Sequence.low.02.mp4

This seems to be a condition where the entire component is re-rendering multiple times due to state or property changes. Can you do a repro of your use case by any means?

@Rajdeepc
This issue is only repoing in Express app. We have an example / development application in which we do not see this issue. I think that the issue has to do with Express being so resource intensive. I think this is a race condition in which the Dialog is not completely finished rendering when the slothandler attaches to the slot content, which forces it to rerender. I think that using he nodes.find delays in just enough that the Dialog completes rendering. The issue can also be fixed using the current slot.assignedElements and a zero wait promise new Promise(resolve => setTimeout(resolve, 0));.

Express does use Service Workers but its still very resource heavy and I think its effecting the rendering of our component. We also use a lot of OverlayTriggers that might effect render changes also.

[zolar]

@Rajdeepc This is a video of the component not rendering. The component container stays blank and you can see the console output going forever. It will eventually crash the browser.
Sequence.low.02.mp4

This seems to be a condition where the entire component is re-rendering multiple times due to state or property changes. Can you do a repro of your use case by any means?

@Rajdeepc This only happens in Express, it does not repo in our development at or any other environment that I know of. There maybe a late state change that is happening but its not from a map or any other iterator that I could find. I think its just a normal state change or its being delayed by the slag in the process.

[coveralls]

Pull Request Test Coverage Report for Build 12362748808

Details

• 30 of 30 (100.0%) changed or added relevant lines in 1 file are covered.
• 12 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.03%) to 98.178%

---

Files with Coverage Reduction	New Missed Lines	%
packages/overlay/src/LongpressController.ts	12	91.67%

Totals
Change from base Build 12278981558:	-0.03%
Covered Lines:	32999
Relevant Lines:	33435

---

💛 - Coveralls