Esri / calcite-design-system

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

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

Open driskull opened 5 months ago

driskull commented 5 months ago

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

KirSpaceB commented 1 month ago

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.