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

USWDS - Input Mask: (Error Feedback) Code Refactor #6203

Open
wants to merge 82 commits into
base: develop
Choose a base branch
from

Conversation

mlloydbixal
Copy link
Contributor

@mlloydbixal mlloydbixal commented Nov 13, 2024

Summary

Improve invalid character feedback with error message. Add error messages (customizable via data attributes) to give proper user feedback when an invalid character is not added to the input value.

Breaking change

This is not a breaking change however there two minor warnings to note:

  • This update adds an error message string below input masks which could result in a small shift in content.
  • For sites with translated content: this update adds error message strings in english only.

Related issue

Closes #5481. Closes #5482.

Related Pull Request

USWDS-Site: Add changelog for input mask [#6203]

Preview link

Preview link

Problem statement

Previously, the input mask component provided minimal visual feedback and no auditory feedback when users attempted to enter an invalid character. Usability testing revealed that users were often unaware that their input was rejected. The goal was to provide clear, accessible feedback through error messages and ARIA attributes, ensuring users understood why their input was rejected and what was expected instead.

Solution

This solution introduced descriptive, dynamic error messages that informed users about invalid inputs and clarified what was expected, whether a letter or a number. Additionally, enhanced ARIA labeling ensured screen readers conveyed the same feedback, with customizable messages that could provide further guidance if needed.

Testing and review

Navigate to text input mask component (alphanumeric is best for testing all the error message types at once but the others should still be tested for regression)

  1. Tab into input, no error should display
  2. Type “A” displays no error
  3. Type “AB” displays “You must enter a number”
  4. Type “A1” displays no error
  5. Type “A12” displays “You must enter a letter”
  6. Type “A1B” displays no error
  7. Finishing input with “A1B 2C3” displays no error
  8. Continuing to type displays error “You have reached the maximum number of characters allowed”
  9. BACKSPACE clears the error
  10. ARROW LEFT and ARROW RIGHT do not trigger an error
  11. CMD + BACKSPACE clears the input with no error displayed
  12. Pasting “A1B2C3” formats text and displays no error
    *Paste tests should only be done with keyboard, issues with paste click events will be covered in a seperate issue
  13. Pasting “A1B2222” displays “Some pasted characters were not accepted.”
  14. Using navigational keys do not display an error: HOME, END, ARROW LEFT, ARROW RIGHT, PAGE UP, PAGE DOWN, TAB
  15. When testing using a screen reader the error message should be read to user 1000ms after the error message displays. This message should be able to be customized separately from the one displayed visually.

*When error message does not display it should also be hidden with no visual padding or styling
*There should be no regressions or changes to mask functionality outside of partial improvements to paste functionality. This is only an error message enhancement.

@mlloydbixal mlloydbixal changed the title Input Mask (Error Feedback) Refactor USWDS - Input Mask: (Error Feedback) Code Refactor Nov 15, 2024
Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Overall this is looking good! I've only tested the functionality so far. Please don't forget to create unit tests.

Discovered a minor quirk in special characters.

For example
Enter ! the error will flash and disappear, but if you enter = it will stay. Attached GIF below.

capture -Arc-2024-11-19

@mlloydbixal
Copy link
Contributor Author

@mejiaj thanks for the draft feedback! Good catch on that bug :O

@mlloydbixal
Copy link
Contributor Author

Overall this is looking good! I've only tested the functionality so far. Please don't forget to create unit tests.

Discovered a minor quirk in special characters.

For example Enter ! the error will flash and disappear, but if you enter = it will stay. Attached GIF below.

capture -Arc-2024-11-19

I think it has something to do with holding down the shift button. I will add this edge case - good catch!

@mlloydbixal
Copy link
Contributor Author

mlloydbixal commented Nov 21, 2024

I'm working on refining the error message so that a screen reader user understands their invalid character was not added to the input. There was some confusion from users if they needed to backspace or not (and they don't need to). I'm wondering if updating the message to say "Error: please enter a number. Your last entry was not added to the value." is descriptive enough OR if we need to adjust functionality to read them back their whole input like "Error: please enter a number. Currently the input value is "a1b 2c"

I notice in VO it reads the new full input value after a character has been accepted, but not when it's rejected.

Thoughts?
@amyleadem @amycole501

@amycole501
Copy link

I prefer your second wording. And in JAWS typically nothing is read aloud for me unless I backspace to hear it. Most JAWS and NVDA users I suspect will be used to arrowing backwards to hear each character. That said, hearing the entire data field announced sounds like a great enhancement!

Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

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

Getting really close! I'm still unable to see the error message on paste and there's one small code polish improvement we can make then it's just onto the unit tests!

Thanks Mandy

Comment on lines +266 to +267
* @return {string} newValue - The full input value after the most recent character is added or rejected.
* @return {string} matchType - The required character type (e.g., 'letter' or 'number') for the current position in the mask.
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: I'm not familiar with the best practice for JSdocs with multiple returns but since this is an object we could use one return and just describe the object returned.

Suggested change
* @return {string} newValue - The full input value after the most recent character is added or rejected.
* @return {string} matchType - The required character type (e.g., 'letter' or 'number') for the current position in the mask.
* @return {object} newValue and matchType strings - The updated value string and matchType for the current position in the mask

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this appears to be closer to how we use JSDocs when larger objects are returned, like our "get context" functions (though this example could use some more detail):

/**
* Get an object of elements belonging directly to the given
* combo box component.
*
* @param {HTMLElement} el the element within the combo box
* @returns {ComboBoxContext} elements
*/
const getComboBoxContext = (el) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I read, the way it is is considered acceptable. An alternative would be something like:

@typedef {object} MaskResult
@property {string} newValue - The full input value after the most recent character is added or rejected.
@property {string} matchType - The required character type (e.g., 'letter' or 'number') for the current position in the mask.
@return {MaskResult}

Any preference?? cc @mejiaj @amyleadem

Comment on lines +362 to +372
// Only part or none of the input was added with copy/paste
} else if (
(inputAddedByPaste && strippedNewValue !== strippedClipboard) ||
(!valueAccepted && inputAddedByPaste)
) {
hiddenStatus = false;
// Set error message content
errorTextContent = errorMsgPaste;
errorMessageSrOnly = errorMsgPasteSrOnly;
// Reset inputAddedByPaste
inputAddedByPaste = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still able to paste invalid characters with the keyboard paste without triggering the error state.

Kapture.2025-02-05.at.13.17.25.webm

It looks like this snippet of code is not being run on paste:

if (newValue.length !== len && inputAddedByPaste) {
newValue = stripPasteValue(newValue, clipboardData);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems rare and not important enough to block! I say we press on for now and if it continues to appear we can look into it down the line

@dct2023 dct2023 linked an issue Feb 6, 2025 that may be closed by this pull request
@dct2023 dct2023 added the Q2 2025 label Feb 7, 2025
@mlloydbixal mlloydbixal requested review from jaclinec and removed request for a team and jaclinec February 12, 2025 16:44
@dct2023
Copy link

dct2023 commented Feb 18, 2025

@mahoneycm This is ready for your review.

@amycole501 amycole501 requested review from heymatthenry and juliaelman and removed request for amyleadem, mejiaj and mahoneycm March 13, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants