Skip to content

Use component ID for input and label to avoid ID collisions#1149

Open
zeroasterisk wants to merge 1 commit intomainfrom
fix/unique-ids
Open

Use component ID for input and label to avoid ID collisions#1149
zeroasterisk wants to merge 1 commit intomainfrom
fix/unique-ids

Conversation

@zeroasterisk
Copy link
Copy Markdown
Collaborator

@zeroasterisk zeroasterisk commented Apr 12, 2026

This PR addresses a bug where the input IDs were hardcoded as data in 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

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator

@ditman ditman left a comment

Choose a reason for hiding this comment

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

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"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

how are you setting this id property? Is it directly in the DOM, or does it come from the agent?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants