Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Audit components to remove getSlotted util instances #6059

Closed
25 tasks done
macandcheese opened this issue Dec 15, 2022 · 8 comments
Closed
25 tasks done

Audit components to remove getSlotted util instances #6059

macandcheese opened this issue Dec 15, 2022 · 8 comments
Assignees
@geospatialem @DitwanP
Labels
4 - verified Issues that have been released and confirmed resolved. estimate - 8 Requires input from team, consider smaller steps. p - medium Issue is non core or affecting less that 60% of people using the library refactor Issues tied to code that needs to be significantly reworked.
Milestone
2024-10-29 - Oct ...

Comments

@macandcheese
Copy link
Contributor

macandcheese commented Dec 15, 2022 •
edited by driskull
Loading

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: #10152

Relevant Info

cc @driskull
@macandcheese macandcheese added refactor Issues tied to code that needs to be significantly reworked. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Dec 15, 2022
@macandcheese macandcheese mentioned this issue Dec 15, 2022
Merged
@driskull
Copy link
Member

driskull commented Aug 3, 2023

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

driskull added a commit that referenced this issue Aug 4, 2023
@driskull
refactor(action-bar, action-pad, action-group): Remove use of getSlot…
7416cc3 
…ted utility. #6059
@driskull driskull mentioned this issue Aug 4, 2023
Merged
driskull added a commit that referenced this issue Aug 4, 2023
@driskull
refactor(shell-panel): Remove usage of getSlotted utility. #6059
905046b
@driskull driskull mentioned this issue Aug 4, 2023
Merged
driskull added a commit that referenced this issue Aug 15, 2023
@driskull
refactor(shell-panel): Remove usage of getSlotted utility. #6059 (#7465)
152fc7d 
**Related Issue:** #6059

## Summary

- Remove usage of getSlotted utility
driskull added a commit that referenced this issue Aug 15, 2023
@driskull
refactor(action-bar, action-pad, action-group): Remove usage of getSl…
4336635 
…otted utility (#7464)

**Related Issue:** #6059

## Summary

- Depends on #7435
benelan pushed a commit that referenced this issue Aug 16, 2023
@driskull @benelan
refactor(shell-panel): Remove usage of getSlotted utility. #6059 (#7465)
7c64627 
**Related Issue:** #6059

## Summary

- Remove usage of getSlotted utility
benelan pushed a commit that referenced this issue Aug 16, 2023
@driskull @benelan
refactor(action-bar, action-pad, action-group): Remove usage of getSl…
563536a 
…otted utility (#7464)

**Related Issue:** #6059

## Summary

- Depends on #7435
benelan added a commit that referenced this issue Aug 17, 2023
@benelan
Merge remote-tracking branch 'origin/main' into benelan/pr-bot-token
389ed0e 
* origin/main:
  chore: release next
  chore: add `directory` to package.json `repository` property (#7510)
  fix(action-bar): restore "bottom-actions" slot functionality. (#7535)
  refactor(action-bar, action-pad, action-group): Remove usage of getSlotted utility (#7464)
  build(deps): update dependency lerna to v7.1.5 (#7502)
  build(deps): update dependency @esri/calcite-ui-icons to v3.23.7 (#7522)
  refactor(shell-panel): Remove usage of getSlotted utility. #6059 (#7465)
  build(deps): update dependency eslint-plugin-unicorn to v46.0.1 (#7501)
  build(deps): replace dependency rollup-plugin-node-resolve with @rollup/plugin-node-resolve 6.0.0 (#7481)
  test: skip unstable tests (#7524)
  build(deps): update dependency lint-staged to v13.2.3 (#7523)
  build(deps): update dependency @types/estree to v1.0.1 (#7483)
  build(deps): update dependency eslint-plugin-react to v7.33.1 (#7500)
  build(deps): update dependency eslint-plugin-jsdoc to v46.4.6 (#7499)
  build(deps): update dependency @types/node to v20.4.10 (#7484)
  chore: release next
  chore: release main (#7485)
@geospatialem geospatialem mentioned this issue Oct 16, 2023
Closed
@jcfranco jcfranco mentioned this issue Oct 16, 2023
Closed
@geospatialem geospatialem added p - medium Issue is non core or affecting less that 60% of people using the library estimate - 8 Requires input from team, consider smaller steps. labels Jan 22, 2024
@geospatialem geospatialem added this to the 2024-08-27 - Aug Release milestone Jan 22, 2024
@geospatialem geospatialem removed the needs triage Planning workflow - pending design/dev review. label Jan 22, 2024
@geospatialem geospatialem mentioned this issue Sep 3, 2024
Closed
6 tasks
@rweber-esri
Copy link

rweber-esri commented Sep 23, 2024 •
edited
Loading

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.

@rweber-esri rweber-esri mentioned this issue Sep 23, 2024
Merged
4 tasks
driskull added a commit that referenced this issue Sep 23, 2024
@driskull
fix(card): properly handle slotted elements. #10152 #6059
c4ba38f
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(combobox-item): fix rendering tied to named-slot content (#10450)
fcd4ed0 
**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event and `@State` variables to update the
display of elements.
- existing tests should suffice
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(notice): fix rendering tied to named-slot content (#10453)
e6fb639 
**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event and `@State` variables to update the
display of elements.
- existing tests should suffice
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(accordion-item): fix rendering tied to named-slot content (#10447)
cc75101 
**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event and `@State` variables to update the
display of elements.
- existing tests should suffice
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(tree, tree-item): fix rendering tied to named-slot content (#10462)
d2f9fb6 
**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event and `@State` variables to update the
display of elements.
- existing tests should suffice
@jcfranco
Copy link
Member

jcfranco commented Oct 1, 2024 •
edited
Loading

@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 added a commit that referenced this issue Oct 1, 2024
@driskull
refactor(action-bar, action-group, action-pad, shell, shell-panel): r…
f2eb65a 
…emove conditional slot component where not necessary (#10464)

**Related Issue:** #6059

## Summary

- removes `ConditionalSlotComponent` and helpers on components that no
longer need them.
- these components do not use getSlotted any longer so these should not
be necessary
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(input, input-number, input-text): should not set slotted actions …
c967333 
…to be disabled (#10458)

**Related Issue:** #6059 #10055

## Summary

- removes getSlotted util usage
- removes setting slotted buttons to be disabled
- removed applicable tests
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(inline-editable): fix rendering tied to default slot content (#10456
7abe7d4 
)

**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event 
- existing tests should suffice
@driskull
Copy link
Member

driskull commented Oct 1, 2024 •
edited
Loading

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

driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(shell-center-row): fix rendering tied to named-slot content (#10451)
aa1e9da 
**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event and `@State` variables to update the
display of elements.
- existing tests should suffice
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(modal): fix rendering tied to named-slot content #6059
bfd26ab
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(tip): fix rendering tied to named-slot content #6059
699f232
This was referenced Oct 1, 2024
Merged
Merged
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(modal): fix rendering tied to named-slot content (#10469)
3586369 
**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event and `@State` variables to update the
display of elements.
- existing tests should suffice
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
chore: remove deprecated utilities and functions. #6059
dc7dd6f
@driskull driskull mentioned this issue Oct 1, 2024
Merged
driskull added a commit that referenced this issue Oct 1, 2024
@driskull
fix(tip): fix rendering tied to named-slot content (#10470)
d95825c 
**Related Issue:** #6059

## Summary

- remove use of `getSlotted` utility
- replace with `slotchange` event and `@State` variables to update the
display of elements.
- existing tests should suffice
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Oct 1, 2024
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned driskull Oct 1, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Installed and assigned for verification.

@DitwanP
Copy link
Contributor

DitwanP commented Oct 29, 2024

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

@DitwanP DitwanP closed this as completed Oct 29, 2024
@DitwanP DitwanP added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Oct 29, 2024
@jcfranco
Copy link
Member

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.

driskull added a commit that referenced this issue Oct 31, 2024
@driskull
refactor: remove deprecated utilities and functions (#10471)
0b37016 
**Related Issue:** #6059

## Summary

- remove `getSlotted()` util and tests
- remove `ConditionalSlotComponent` utility file.
- remove `getElementProp()` util. Unused.
- depends on removal of `pick-list` and `value-list` components.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

@DitwanP DitwanP

Labels
4 - verified Issues that have been released and confirmed resolved. estimate - 8 Requires input from team, consider smaller steps. p - medium Issue is non core or affecting less that 60% of people using the library refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Milestone
2024-10-29 - Oct Milestone
Development

No branches or pull requests
7 participants
@jcfranco @driskull @alisonailea @macandcheese @geospatialem @rweber-esri @DitwanP