spectrum-web-components

spectrum-web-components copied to clipboard

Could we made SideNavItem.prototype.hasChildren be

    get hasChildren(): boolean {
        return !!this.querySelector('[slot="descendant"]');
    }

So that all kinds of descendants will be accepted as children of a side-nav-item, for example, a custom derived component.

Originally posted by @OpportunityLiu in https://github.com/adobe/spectrum-web-components/issues/1671#issuecomment-900995133

Pros:

1. Allow custom derived components works correctly with unmodified components.
2. Make it possible to use a different tag name (To avoid conflictions?)
3. Maybe it is possible to find elements only by DOM structures and slot property, so that all elements can be accepted. (It's the user's responsibility to make these combinations works correctly, we can just regard it works and use these elements as what we did before)

Cons:

1. Worse performance (Maybe? not tested)
2. If all elements accepted, some components will "work incorrectly" instead of "not work", with wrong DOM structure, which leads to confusion.

@OpportunityLiu this is a great question for us to get sorted. To support in the architectural decisions here, would you happen to have any examples where you've created custom alternatives to our elements to better understand the problem space you encounter in this context?

Also, we're looking at investing in the Scoped Element Registries API (and it's polyfill) in the near future. In the case that something like this was more fully supported by the library would be you that as a path towards being able to freely extend the capabilities of these sorts of children without needing to customize their tag name? Off hand, the place where I see this still creating issue is if you were leverages something like our exact implementation of sp-sidenav-item side by side with an liu-sidenav-item implementation in the same sp-sidenav parent, but understanding if that's "expected" or "edge case" will help us when we move into investigating options here.

Some notes that I can think would also be useful when investigating this:

• In some cases we "polyfill" Imperative Slot Distribution by having elements apply their own slot attribute (Sidenav, Tabs, etc.) that can change the timing within the element lifecycle when certain selectors will be true: e.g. [slot="descendant"] isn't guaranteed to be available at first run.
• Some elements leverage closest(...) to query up. There's room to say this is required to be strict, but there may be room for thinking through ways to leverage instanceof (requires version matching) or available API checks (see Base.js) to ensure "equality".
• sp-menu uses an event system to manage its children sp-menu-item descendants that might be of benefit in these cases, the majority of which should be many, many times simpler than the fairly complex processing leveraged therein.

Currently, I'm using custom derived components to add some additional styles and modify render functions.

For example, in sidenav-item, I'm using following styles to force single line label for long content, use content-visibility and contain-intrinsic-size to optimize render performance for huge lists (It's extremely easy to apply rather than implementing virtual scrolling). In the ts file, I override some methods to support some situation that the sidenav-item is rendered by a list component in its shadow dom, so that parentSideNav and depth should be computed cross shadow roots.

#itemLink {
  align-items: center;
  display: flex;
  padding: 0 var(--spectrum-sidenav-item-padding-x, var(--spectrum-global-dimension-size-150));

  slot::slotted(*) {
    flex: 0 0 auto;
  }
}

#label {
  overflow: hidden;
  text-overflow: ellipsis;
  white-space: nowrap;
}

:host {
  contain-intrinsic-size: var(--spectrum-sidenav-item-height, var(--spectrum-alias-single-line-height));
}

:host(:not([expanded])) {
  contain: content;
  content-visibility: auto;
}

Another example is sp-dialog, as you motioned in #1088, a fullscreen dialog cannot be dismissable. But I really wants a dismissable fullscreen dialog, so I overrides its shouldUpdate methods and full screen grid-template-areas.

To dos:

• [ ] Action Group uses a button selector: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/action-group/src/ActionGroup.ts#L37
• [ ] Action Group queries same name siblings: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/action-group/src/ActionGroup.ts#L193
• [ ] Menu filters children by localName: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/menu/src/Menu.ts#L613
• [ ] Picker warns deprecation on sp-menu: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/picker/src/Picker.ts#L249
• [ ] Picker queries children: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/picker/src/Picker.ts#L416
• [ ] Radio Group queries same name siblings: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/radio/src/RadioGroup.ts#L127
• [ ] Radio Group uses specific checked child: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/radio/src/RadioGroup.ts#L214
• [ ] Sidenav does this a bunch, starting with: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/sidenav/src/Sidenav.ts#L117
• [ ] Sidenav Item queries up and down by element name: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/sidenav/src/SidenavItem.ts#L52
• [ ] Tags queries same name siblings: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/tags/src/Tags.ts#L125
• [ ] Top name queries child items: https://github.com/adobe/spectrum-web-components/blob/9ab5f05eacbe11e30cbf7158b032733c781f1104/packages/top-nav/src/TopNav.ts#L85