Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
295 stars 76 forks source link

Audit components to remove `getSlotted` util instances #6059

Closed macandcheese closed 1 month ago

macandcheese commented 1 year ago

Description

Many components use getSlotted and will need to be refactored in favor of a new pattern.

Proposed Advantages

The getSlotted util is no longer a recommended pattern - onSlotChange is preferred.

Which Component

Note: prioritize card effort in the initial stages first, if at all possible -- related issue: https://github.com/Esri/calcite-design-system/issues/10152

Relevant Info

cc @driskull

driskull commented 1 year ago

This one would be nice to get onto a milestone sometime in the next year.

rweber-esri commented 2 months ago

See my comment on 10152. Hub is observing certain slotted contents not working in calcite-card even when we immediately slot the content into the component during initial component render on 2.12.1. This is observed when rendering calcite-card from other Stencil components (JSX) Hub internally develops/maintains that are lazy loaded (may play a factor) in our Ember application. Not sure if that influences priority, but we seem to be unable to reliably slot certain content in calcite-card at this time.

driskull commented 2 months ago

@geospatialem @DitwanP besides card which are next highest priority? Should we prioritize this list for future implementation?

jcfranco commented 2 months ago

@driskull's been crushing it with this audit!

Some of the associated PRs are fixes, so we might need to adjust this issue to accommodate for testing and verification. cc @geospatialem @DitwanP

driskull commented 2 months ago

Should we update modal and tip since they will be around till v4?

github-actions[bot] commented 2 months ago

Installed and assigned for verification.

DitwanP commented 1 month ago

🍠✨ Verified. The e2e and screenshot tests does indeed provide good coverage for this! 💪

jcfranco commented 1 month ago

Agreed. We can also keep an eye out for anything our testing may have missed. For context, the fix-side on the related PRs addresses the point from this PR:

The conditionalSlot utility class will not work if ComponentA slots into a slot in ComponentB.