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

Deprecate public guid properties on checkbox and radio-button #9110

Closed
1 of 5 tasks
driskull opened this issue Apr 10, 2024 · 3 comments
Closed
1 of 5 tasks

Deprecate public guid properties on checkbox and radio-button #9110

driskull opened this issue Apr 10, 2024 · 3 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. calcite-components Issues specific to the @esri/calcite-components package. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. good first issue Issues that can be worked on by contributors new to calcite-components. p - low Issue is non core or affecting less that 10% of people using the library refactor Issues tied to code that needs to be significantly reworked.

Comments

@driskull
Copy link
Member

Description

Make these properties internal private variables instead of public properties.

Rely on an id being set on the host of the component or generate a guid and set it as an id on the host if not defined.

Proposed Advantages

No unique guid property
Currently, the guid is typed as guid() which isn't ideal on the docs

Which Component

checkbox
radio-button

Relevant Info

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components
@driskull driskull 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. good first issue Issues that can be worked on by contributors new to calcite-components. labels Apr 10, 2024
@github-actions github-actions bot added the calcite-components Issues specific to the @esri/calcite-components package. label Apr 10, 2024
@KirSpaceB
Copy link

Current Implementation:

//initialized guid property under private properties section:
guid:string
//created guidMutate method under private methods section to replicate the mutate property stenciljs provided:
guidMutate = (value:string) => {
   this.guid = value;
   this.el.id = value;
}

// added "extends HTMLElement" to HTMLCalciteCheckboxElement in order to access setAttribute method for the reflect logic.
connectedCallback(): void {
    this.guid = this.el.id || calcite-checkbox-${guid()};
    this.el.setAttribute('guid', this.guid);
    connectInteractive(this);
    connectLabel(this);
    connectForm(this);
  }

I'm still a bit confused on why properties don't have the private keyword. Additionally, none of the other properties extend HTMLElement so I could just be misunderstanding the current convention in the project. Any feedback is appreciated.

@geospatialem geospatialem added p - low Issue is non core or affecting less that 10% of people using the library estimate - 2 Small fix or update, may require updates to tests. breaking change Issues and pull requests with code changes that are not backwards compatible. future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. and removed needs triage Planning workflow - pending design/dev review. breaking change Issues and pull requests with code changes that are not backwards compatible. estimate - 2 Small fix or update, may require updates to tests. labels Sep 26, 2024
@geospatialem geospatialem changed the title remove public guid properties on checkbox and radio-button Deprecate public guid properties on checkbox and radio-button Sep 26, 2024
@driskull driskull self-assigned this Sep 30, 2024
@driskull driskull added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Sep 30, 2024
driskull added a commit that referenced this issue Sep 30, 2024
**Related Issue:** #9110

## Summary

- deprecate guid properties
@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 Sep 30, 2024
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned driskull Sep 30, 2024
@DitwanP
Copy link
Contributor

DitwanP commented Sep 30, 2024

🍠✨

@DitwanP DitwanP closed this as completed Sep 30, 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 Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. calcite-components Issues specific to the @esri/calcite-components package. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. future breaking change Issues and pull requests with deprecation(s), requires a breaking change in a future milestone. good first issue Issues that can be worked on by contributors new to calcite-components. p - low Issue is non core or affecting less that 10% of people using the library refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

No branches or pull requests

4 participants