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.

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

feat(list-item): deprecate open/close props in favor of expanded/collapsed #11003

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Dec 6, 2024

Related Issue: #6473

Summary

Deprecate open/close props in favor of expanded/collapsed.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Dec 6, 2024
* When `true`, a close button is added to the component.
*
* @deprecated Use `collapsible` prop instead.
*/
@property({ reflect: true }) closable = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be collapsible. It isn't collapsed, it isn't shown at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think closed/closable needs to remain as is.

@@ -357,20 +378,32 @@ export class ListItem
To account for this semantics change, the checks for (this.hasUpdated || value != defaultValue) was added in this method
Please refactor your code to reduce the need for this check.
Docs: https://qawebgis.esri.com/arcgis-components/?path=/docs/lumina-transition-from-stencil--docs#watching-for-property-changes */
if (changes.has("expanded")) {
this.open = this.expanded;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. expanded should set expanded, open should set open.

I think what needs to happen is that open and expanded each need a custom setter to set the other one when one is set.

Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. Stale Issues or pull requests that have not had recent activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants