fix(lemon-ui): wire LemonField htmlFor so clicking labels focuses inputs#60548
Merged
Conversation
LemonField rendered every label with an empty `htmlFor`, so clicking a label did nothing. The Kea-Forms template never threaded an id into `LemonPureField`, and when callers passed a render-prop child (e.g. the signup "Where did you hear about us?" field), the underlying `LemonInput` was never given an `id` either — both ends of the label/input association were missing. Generate a stable id with `useId()` in `LemonField`, prefer any caller-provided `htmlFor` and any id kea-forms already injected onto the child, otherwise fall back to the generated id. Clone the rendered kids so the input gets the same id we use for the label's `for` attribute. Adds tests covering Pure usage, function-as-child render props, and React-element children so this regression can't return silently. Generated-By: PostHog Code Task-Id: 4f712b5a-e63c-4f85-b54e-c73e6f0dcd99
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
frontend/src/lib/lemon-ui/LemonField/LemonField.test.tsx:53-66
**Fragile DOM selector may break if LemonInput internals change**
`document.querySelector('input.LemonInput__input')` couples the test to an implementation-internal CSS class. If `LemonInput` renames that class, this test silently breaks. Prefer `screen.getByRole('textbox')` (or `screen.getByRole('textbox', { name: /where did you hear/i })`) which relies on accessible role semantics instead, and is consistent with the `@testing-library` style used elsewhere in the project. The same applies to the identical selector in the third test (line 75).
### Issue 2 of 2
frontend/src/lib/lemon-ui/LemonField/LemonField.test.tsx:44-82
**Tests could be parameterised per the project convention**
The three `it` blocks all verify the same invariant — "label `for` equals the input `id`" — under different setup conditions. Per the project's convention of preferring parameterised tests, wrapping the assertions in an `it.each` (or at minimum combining the two kea-forms cases) would make it easier to add new scenarios without copy-pasting the boilerplate.
Reviews (1): Last reviewed commit: "fix(lemon-ui): wire LemonField htmlFor s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
Size Change: 0 B Total Size: 80.9 MB ℹ️ View Unchanged
|
Switches to `screen.getByLabelText` to assert usage is proper
rafaeelaudibert
approved these changes
May 29, 2026
The inline `kea<any>` test logic made kea-typegen rewrite the source in CI (adding the generated type import and typed `kea<...>` call), which left the working tree dirty and failed the "schema/validators up to date" check. Match the committed source to what typegen emits so the check stays clean. The generated `*Type.ts` file itself remains gitignored and is produced by typegen at build time. Generated-By: PostHog Code Task-Id: 4f712b5a-e63c-4f85-b54e-c73e6f0dcd99
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
LemonFieldrendered every label with an emptyhtmlFor, so clicking a label did nothing — a dead-click pattern visible in session replay across the signup form and any other Kea-Forms field built withLemonField. PostHog inbox surfaced this from rage-clicks on the "Where did you hear about us?" label of the org-creation form.The Kea-Forms template never threaded an id into
LemonPureField, and for render-prop children ({({ value, onChange }) => <LemonInput ... />}) the underlyingLemonInputwas never given anideither, so both ends of the label/input association were missing.Changes
LemonFieldnow generates a stable id withuseId()and exposes it as the label'shtmlForviaLemonPureField.htmlForprop, (2) any id kea-forms has already injected onto a React-element child, and (3) the generated id as a fallback for the render-prop case.idmatches the label'sfor—LemonInputalready forwardsidto the underlying<input>, so the standard browser label-focus behavior is restored everywhereLemonFieldis used.No call-site changes required: forms using either form (React-element child or render-prop) are fixed by the template.
How did you test this code?
I'm an agent (PostHog Code). Only automated tests were run, no manual UI verification:
frontend/src/lib/lemon-ui/LemonField/LemonField.test.tsxcovering:LemonField.Purepassing an explicithtmlForthrough to the label.LemonInputends up with an id matching the label'sfor.pnpm jest frontend/src/lib/lemon-ui— 13 suites, 146 tests pass.tsc --noEmitfor the changed files surfaces no new errors.Automatic notifications
🤖 Agent context
Authored by PostHog Code in response to inbox report
019dfe02-f4b5-7077-9583-ec2394ae30bf.Investigation:
LemonField.tsx(Kea-Forms template never propagatedhtmlFor) and noted thatSignupReferralSource.tsxis one of many affected call sites.Fieldsource (3.2.0) to confirm it already computes an id for React-element children viacloneElementbut does not inject ids for function-as-child render props. That asymmetry shaped the resolution order in the fix.Considered approaches:
idexplicitly insideSignupReferralSource) — leaves the rest of the product still broken and easy to regress.LemonFielditself so every consumer benefits with no API change. Cloning the rendered kids is safe because consumers that already supply an id keep theirs (precedence check in the resolver).This PR requires human review before merge.