Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TextField error message inheriting label styles #1219

Open
Josh68 opened this issue Nov 9, 2021 · 4 comments
Open

TextField error message inheriting label styles #1219

Josh68 opened this issue Nov 9, 2021 · 4 comments

Comments

@Josh68
Copy link
Collaborator

Josh68 commented Nov 9, 2021

Describe the bug
On MCT, we recently found that the TextField component -- which is using the FormControl component underneath, renders hiddenError markup for the prop errorMessage, and that hiddenError renders as a span inside the rendered label element. Therefore, if you set a labelClassName, the error message adopts the label's styling (e.g., in our case, increased font size). You can override this by setting an errorMessageClassName that addresses the same property or properties as labelClassName, but this seems unintuitive.

Would it be spec-compliant and not too difficult to render the hiddenError span instead between the label and input, to avoid unintended style inheritance? On MCT, the UX team has specified label styling that they don't want carried to error messages, at least in some places, so our options are to deal with this on a case-by-case basis or create our own TextField component that wraps the one from the DS and always sets errorMessageClassName to be ds-u-font-size--base, which is our UX standard.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://www.medicare.gov/plan-compare/#/questions?year=2022&lang=en
  2. Select the "Medicare Advantage" radio button
  3. Type in an invalid value in the Zip Code input (e.g., 123) and select "Continue"
  4. See the error displayed. As of the time you do this, you may need to go into dev tools and remove ds-u-font-size--base from the rendered error message span
  5. See how the error message by default would inherit styles from the label

Expected behavior
The error message's styling is handled independently of the input label's

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • Mac OS 10.15.7
  • Chrome, FF, or Safari

Smartphone (please complete the following information):

  • N/A

Additional context
Add any other context about the problem here.

@pwolfert
Copy link
Collaborator

pwolfert commented May 9, 2022

Doing a little research here. It looks like this is the relevant bit of code:
https://github.com/CMSgov/design-system/blob/master/packages/design-system/src/components/FormLabel/FormLabel.tsx#L139-L143

The hidden error here is only rendered when the error is being placed below the input, and it is only visible to screen readers, so I don't think it comes into play in the example scenario above.

In both scenarios, some form of the error message needs to always be rendered inside the <label> element for a11y purposes. The ask, then, would be for there to be a way to apply css classes to the span that contains visible label text only, separate from the hint text or the error text. This might be rather difficult to change, actually, because we don't know what all our users are doing with that labelClassName and if they need it to apply to the overall label container. We'd need to do some research, and it would probably be either a breaking change or involve adding a new, more specific prop for that span. This is not a decision; this is just me writing down the results of my research.

@forrestbaer
Copy link
Collaborator

Checking in on this one, @pwolfert was this touched on in the latest FormLabel discovery you and @zarahzachz were doing? Would we consider this a won't do or perhaps I can help by getting a ticket in the backlog where we can track this further?

@pwolfert
Copy link
Collaborator

@forrestbaer, it looks like this might actually get fixed by the changes @zarahzachz has in her branch related to that discovery. We should keep this in mind and probably link to it in that discovery ticket, but I don't think we need a separate Jira ticket just yet. Thanks for identifying the link!

@forrestbaer
Copy link
Collaborator

Sounds good! @Josh68 it looks like #2444 is where this discovery is happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants