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 - Select: Multiple select overflow #5268

Merged
merged 8 commits into from
Jul 28, 2023

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented May 2, 2023

Summary

Added ellipses to overflow text in the multiple variant of the select component. This provides a clear indication to users that there is text that extends beyond the select width.

Breaking change

This is not a breaking change.

Related issue

Closes #5047

Related pull requests

Site Changelog PR

Preview link

Multiple select →

Problem statement

Multiple select cuts off long options on Safari & Firefox due to browser styles changing the text color to white, and additional padding leftover from the single select's dropdown icon

Solution

Unfortunately, there is no way to wrap option text within a select element at this time. Instead, the component should convey that there is additional text not visible to the user. This will provide a better user experience for all multiple select users.

This still isn't ideal for users and developers should focus on allowing options to be completely visible within their selects by either increasing the select element width or using short, concise options.

Testing and review

  1. Visit multiple select story in Safari or Firefox
  2. View the longer Option C
  3. Confirm ellipses are visible
  4. Select option C
  5. Confirm no information loss due to padding or text color change

Screenshots

Before

Chrome:
image

Firefox
image

Safari
image

After

Chrome:
image

Firefox
image

Safari
image

  • Confirm that this code follows the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run git pull origin [base branch] to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch is develop).
  • Run npm run prettier:scss to format any Sass updates.
  • Run npm test and confirm that all tests pass.

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

@mahoneycm, this seems to resolve the issue, but unfortunately I was not able to see the ellipsis on Safari. I've added some notes below.

Here are some of the things I tested:

  • On init, confirm ellipsis is present on long option text
    • I do not see the ellipsis in Safari. I do see the ellipsis in Firefox and Chrome.
      Safari (initialized):
      image
  • On focus, confirm background color extends the full width of the input
    • In Safari, the background does not extend the full-width of the text. Chrome and Firefox look good.
      image
      image
  • Confirm screen reader reads entire text
  • Tested on Firefox, Safari, Chrome
    • Note, there is some behavior in Firefox that I didn't expect, but it appears to be native behavior in select boxes with overflow-x: hidden options.
      • If I hit tab once, focus moves to the entire input (Expected)
      • If I hit tab again while focus is on the input, it focuses on the option with overflow text. (Not expected, for me at least)
      • Some quick testing revealed that this happens in non-USWDS/standard multiple select boxes as well. Not sure if there is any action that can be taken here, just wanted to flag it in case anyone else noticed or if there is a workaround.

@mahoneycm
Copy link
Contributor Author

Update

Elipsis on Safari

Unfortunately, it looks like Safari doesn't support text-overflow: ellipsis on <option> tags. I looked around for solutions and tried playing with it myself for a while but it doesn't seem possible without JS.

We can either remove it so it's consistent across the board or keep it for Chrome and Firefox

Removed all of the right-side padding

Now that we can't rely on the ellipses to stop the text from reaching the edge, I've removed the right side padding completely to allow the focus background to reach the edge of the select element.

Results:

Chrome:
image

Safari:
image

Resolved Firefox keyboard behavior

Switching overflow-x to overflow. This resolved the tabbing issue where Firefox would target overflow options after targetting the select box itself

@mahoneycm mahoneycm requested a review from amyleadem May 16, 2023 18:34
@mahoneycm
Copy link
Contributor Author

@mejiaj @amyleadem Bumping for re-review once you all have availability after 3.5.0 work!

Copy link
Contributor

@amyleadem amyleadem 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 to me! I performed the following tests:

  • Move focus to the long text option item - Confirm content and background color extends the full width of the input
  • Select the long text option item by moving focus to it then move focus out of the component - Confirm that the background color shows behind only the selected text and that the background color extends the full width of the input
  • Tested on Firefox, Safari, Chrome

@mahoneycm
Copy link
Contributor Author

@mejiaj bumping for review !

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.

LGTM! I tested the following:

Visual crossbrowser testing

  • Confirmed ellipsis in Chromium/Firefox.
  • Confirmed Safari doesn't ― which is OK since this was flagged in description.

Code

Code looks good, no issues. It's unfortunate that the component is hardcoded HTML, so the template isn't reusable. That's not a blocker for this though ― and it'd require a new JSON file anyways. Also, unsure of removing padding right. Either way it doesn't make a huge impact. Just flagging these concerns.


@mahoneycm could you please update the PR description so the information/links are accurate?

Broken preview link

image

Site PR link confusing

Site PR link is showing as closed:
image

There's a comment at the bottom that links it to an already merged 3.5.0 PR.
image

@mahoneycm mahoneycm requested a review from mejiaj July 5, 2023 14:53
@mahoneycm
Copy link
Contributor Author

@mejiaj thanks for flagging! Updated PR description and reopened changelog PR.

If we want to adjust the right padding instead of getting rid of it completely, we could do something like this:

image

Lmk what you think!

@mahoneycm
Copy link
Contributor Author

@mejiaj It does result in odd styles for Safari

image

image

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.

PR description is accurate & up-to-date. Approved!

@thisisdano thisisdano merged commit d5a7fb0 into develop Jul 28, 2023
@thisisdano thisisdano deleted the cm-multiple-select-overflow branch July 28, 2023 17:10
@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 - Bug: Multiple Select (component) option text overflow
4 participants