search

adobe / spectrum-web-components

Spectrum Web Components
https://opensource.adobe.com/spectrum-web-components/
Apache License 2.0
1.24k stars 199 forks source link

Avoid using tag names to find elements #1719

Open OpportunityLiu opened 3 years ago

OpportunityLiu commented 3 years ago

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.
Westbrook commented 3 years ago

@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:

OpportunityLiu commented 3 years ago

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;
}
OpportunityLiu commented 3 years ago

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.

Westbrook commented 2 years ago

To dos: