Use component ID for input and label to avoid ID collisions#1149
Use component ID for input and label to avoid ID collisions#1149zeroasterisk wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded 'data' IDs and label associations with dynamic values using this.id in the DateTimeInput and TextField components. The reviewer identified that using this.id directly causes ID collisions with the host element and suggested using a derived ID like ${this.id}-input instead. Additionally, the reviewer noted that the name attribute remains hardcoded and requested the inclusion of tests for these changes.
c44c78c to
2d7eca8
Compare
… Lit input components
2d7eca8 to
e849e3f
Compare
ditman
left a comment
There was a problem hiding this comment.
Please update the ## Unreleased section of the CHANGELOG with a description of this change.
Also, please consider adding this functionality to the other renderers so we don't have diverging capabilities in the basic catalog components!
(Also, also: only land this if it's a super critical need! I'd rather have this fix in the v0.9 stack :))
| > | ||
| <label | ||
| for="data" | ||
| for="${this.id}-input" |
There was a problem hiding this comment.
how are you setting this id property? Is it directly in the DOM, or does it come from the agent?
This PR addresses a bug where the input IDs were hardcoded as
datain the Lit renderer, which made browser automation difficult.I made what I believe are reasonable changes, but since Paul wrote the initial version of this, I seriously doubt my changes and worry that his initial version is better. I defer to @paullewis but present this as a possible PR