1189/menu removal spike

[adixon-adobe]

Description

This is just a quick spike on changing the re-parenting for picker so sp-menu isn't necessary.

Related Issue

#1189

Motivation and Context

Not looking to merge this. Opening this to discuss the implications of this change.

[adixon-adobe]

To our discussion earlier about adding a mixin that manages the reparenting, I'm trying to think a bit about how that would work.

I could see doing this in a couple of ways, but am curious to get your thoughts.

    this.beginReparentContent(targetElements, newParent) // do the reparenting & add placeholders
    this.endReparentContent() // puts everything back

Though rather than a mixing, maybe just a class that manages the above would be simpler?

   this.reparentManager = new ReparenterManager()
   // same behavior as above here
   this.reparentManager.begin(targetElements, newParent)
   this.reparentManager.end()

Also, I'm definitely not naming things well today...

[adixon-adobe]

I think we'll want to keep the existing usage backward-compatible for now. So here we'd look for an sp-menu and just reparent the children of that (and I was thinking of adding a console.warn DEPRECATION notice for that).

[Westbrook]

Beyond the functional failures, this branch has some new a11y failures (https://app.circleci.com/pipelines/github/adobe/spectrum-web-components/5481/workflows/768f1f48-9152-4f38-a045-01da666c3a26/jobs/45167/parallel-runs/0/steps/0-105) due to how sp-menu-item currently self applies role attributes 100% of the time: https://github.com/adobe/spectrum-web-components/blob/main/packages/menu/src/MenuItem.ts#L107 we'd need to address that as well to do forward with this approach. Interestingly enough this is managed in sp-picker and extensions at current, so you're already in the right place.

[Westbrook]

Being you've chosen to make this an object oriented API would it make better sense to recycle the reparenter rather than newing it every time?

I could alternatively see it as a functional API, a la:

import { reparentChildren } from 'shared';

this.reclaimChildren = reparentChildren(children, target); 

if (this.reclaimElements) {
    this.reclaimChildren();
    this.reclaimChildren = undefined;
}

I know right, a web component admiring suggesting a functional API... 😏

[Westbrook]

In the realm of a class, you will find many parallels with reactive controller approach that the Polymer team is formalizing for the upcoming release of lit-element: https://github.com/Polymer/lit-html/blob/lit-next/packages/reactive-element/CHANGELOG.md

[adixon-adobe]

The functional approach definitely seems better here, and I even tried that first. I was passing back state and not a callback though which I didn't like.

[Westbrook]

This seems to be on the right track. Would be interested in how this API supports overlay-trigger or not as a confirmation of the approach.

[adixon-adobe]

I think it should be pretty easy to rework the core reparenting logic here to use the same helper: https://github.com/adobe/spectrum-web-components/blob/main/packages/overlay/src/ActiveOverlay.ts#L328

I also know we were talking about potentially supporting an element array for the overlay contents, but that looks a lot more disruptive (e.g. it looks like there's some delayed attribute handling on the content element).