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: Fix hover state #5378

Merged
merged 5 commits into from
Aug 11, 2023
Merged

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Jul 13, 2023

Summary

Fixed a bug in input mask that caused the hover state to show disabled styling. This change also improved the legibility of the component in forced colors mode.

Breaking change

This is not a breaking change.

Related issue

Closes #5376

Related pull requests

Changelog PR

Preview link

Preview link: Input mask

Problem statement

  1. The input mask component mistakenly receives disabled styles on hover.

  2. The input mask is not legible in forced colors mode. The usa-input-mask--content element hides the input text. Example:

    image

Solution

This solution fixes two separate items:

  1. Removed the disabled styles on hover in default view mode. This issue originated in PR USWDS - Disabled styles: Improve consistency in high contrast mode #5295 in this commit. The u-disabled mixin appears to have been mistakenly added to the default component state.
  2. Made the component legible in forced colors mode. This was done by making the text inside usa-input-mask--content visible.

    Note
    The experience in forced colors mode works but is not perfect. The cursor is mostly hidden behind usa-input-mask--content. Because this is a high priority issue, the style issue is small, and the path for fixing is not clear, I recommend that if we want to fix this we should address these styling tweaks in a follow-on issue.

Testing and review

In standard viewing mode:

  1. Confirm the hover state does not show disabled styles
  2. Confirm that disabled and aria-disabled states look as expected

In forced colors mode:

  1. Confirm the component visually shows the text added to the input
  2. Confirm the hover state does not show disabled styles
  3. Confirm that disabled and aria-disabled states look as expected

@amyleadem
Copy link
Contributor Author

amyleadem commented Jul 14, 2023

@mahoneycm I had a (hopefully) quick question for you about forced colors mode. In its current state, I am able to make the component mostly legible in forced color mode, but to do that I had to ⁠turn on visibility of this element.

This is a deviation from standard behavior because in forced colors mode the usa-input-mask--content element covers up the default input content with what appears to be a background color. This feels like a workable solution but is not ideal because the cursor is mostly hidden.

All that to say - do you know if there is a way to prevent usa-input-mask--content from covering up the typed text in the input?

Let me know if you have any questions.

Update: During discussion, we determined that the style issue does not warrant blocking progress on this PR.

- This prevents input text from peeking behind usa-input-mask--content
@amyleadem amyleadem marked this pull request as ready for review July 18, 2023 21:36
@amyleadem amyleadem requested review from mejiaj and mahoneycm July 18, 2023 21:45
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.

Looks good! Thanks for taking this on. I created issue #5396 to resolve the disabled input styling by default in high contrast mode. I'll have a fix up shortly!

  • Default and disabled variants look great in standard views
  • Input text is visible in hc mode
  • Hover does not activate disabled state
  • Color matches standard input colors
  • Functionality vastly improved

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.

Looks good! I've tested both standard viewing mode and forced colors.

Standard mode

  • Disabled styles on hover regression is gone.
  • Both aria-disabled and disabled look good.
  • No visual regressions. Compared this preview to develop and release-3.4.1.

Forced colors mode

  • Disabled styles aren't showing on hover.
  • Component visually looks good.
  • Both aria-disabled and disabled look good.

Copy link
Member

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

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

Good fix, thinking, and prioritization for this important bug

@thisisdano thisisdano merged commit daf1956 into develop Aug 11, 2023
@thisisdano thisisdano deleted the al-input-mask-disabled branch August 11, 2023 16:02
@thisisdano thisisdano mentioned this pull request Aug 18, 2023
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

Successfully merging this pull request may close these issues.

USWDS-Site - Bug: Input mask example shows wrong cursor on hover
4 participants