fix(form-control): import FormControlHelperText from the correct file#3422
Open
patrickwehbe wants to merge 1 commit into
Open
fix(form-control): import FormControlHelperText from the correct file#3422patrickwehbe wants to merge 1 commit into
patrickwehbe wants to merge 1 commit into
Conversation
FormControl.Helper.Text was being built from the helper wrapper, not the
helper-text wrapper, because the creator imported FormControlHelperText
from './FormControlHelper'. The helper wrapper forces id={labelId} and
calls setHasHelpText, so the help text ended up with the label's id while
aria-describedby still pointed at the help text id, which no element had.
That breaks the description association for screen readers (and trips axe's
aria-valid-attr-value, same failure mode as gluestack#3410 on Radio.Group).
The correct FormControlHelperText component already exists and is a plain
pass-through; just import it from './FormControlHelperText', matching the
FormControlErrorText import right above it.
Contributor
|
Someone is attempting to deploy a commit to the GeekyAnts Labs Team on Vercel. A member of the Team first needs to authorize it. |
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
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.
What
packages/gluestack-core/src/form-control/creator/index.tsximports the helper-text component from the wrong file:So
FormControl.Helper.Textgets built from the helper wrapper (FormControlHelper) instead of the helper-text wrapper. The error side one line above already does it right (FormControlErrorTextfrom./FormControlErrorText); this just makes the helper side match.Why it matters (a11y)
FormControlHelperis not a neutral pass-through. It does two things the helper text should not:Because of the wrong import:
id={labelId}, so it renders with the label's id. ButuseFormControlbuildsaria-describedbyfromhelpTextId(${id}-helptext). Nothing in the DOM has that id, so the input'saria-describedbypoints at a non-existent element and the help text is never announced. axe flags this asaria-valid-attr-value(same failure mode reported for Radio.Group in Radio.Group (web): hidden radio inputs getaria-describedbyreferencing a non-existent element — axe flagsaria-valid-attr-value#3410).setHasHelpTextfires twice, once fromFormControl.Helperand again fromFormControl.Helper.Text, so the context flag gets toggled by an element that shouldn't own it.The real
FormControlHelperText.tsxalready exists and is the correct thin wrapper (<StyledFormControlHelperText ref={ref} {...props}>), it was just never imported.Fix
One line: import
FormControlHelperTextfrom./FormControlHelperText.Verifying
I rendered the two wrappers in jsdom with a realistic FormControl context (
id=field-1,labelId=field-1-label,helpTextId=field-1-helptext):field-1-label,setHasHelpTextfires from itfield-1-helptext, no extrasetHasHelpTextBoth wrappers export the same
(Styled) => forwardRef(...)shape, so theIFormControlComponentTypetyping is unaffected and the change is just behavioral.Changeset included (patch on
@gluestack-ui/core).