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

Cannot set width of va-select input #2644

Open
2 of 6 tasks
josephrlee opened this issue Mar 29, 2024 · 10 comments
Open
2 of 6 tasks

Cannot set width of va-select input #2644

josephrlee opened this issue Mar 29, 2024 · 10 comments

Comments

@josephrlee
Copy link
Contributor

Bug Report

  • I’ve searched for any related issues and avoided creating a duplicate issue.

What happened

Can't set the width of a va-select component width without using custom CSS.

Related ticket: #1712

What I expected to happen

Ideally, I would like to be able to have a width prop like is possible on the va-text-input.

Reproducing

  • Formation version:
  • Device: Macbook Pro
  • Browser: Chrome

Steps to reproduce:

  1. Create a va-select or VaSelect
  2. Try to change the width to a small size in the code

Urgency

How urgent is this request? Please select the appropriate option below and/or provide details

  • This bug is blocking work currently in progress
  • This bug is affecting work currently in progress, but we have a workaround
  • This bug is blocking work planned within the next few sprints
  • This bug is not blocking any work
  • Other

Details

Here's what we've implemented in the FSR to accomplish this: https://github.com/department-of-veterans-affairs/vets-website/blob/4ed97e5e0438d2f45e66ba4c1f94568d36cdb738/src/applications/financial-status-report/components/employment/EmploymentRecord.jsx#L127

<div className="input-size-5">
        <VaSelect
          name="type"
          data-test-id="employment-type"
          label="Type of work"
          required
          value={employment[index].type || []}
          onVaSelect={e => handleChange('type', e.detail.value)}
          error={showError() || null}
          uswds
        >
          <option value=""> </option>
          <option value="Full time">Full time</option>
          <option value="Part time">Part time</option>
          <option value="Seasonal">Seasonal</option>
          <option value="Temporary">Temporary</option>
        </VaSelect>
      </div>

https://github.com/department-of-veterans-affairs/vets-website/blob/4ed97e5e0438d2f45e66ba4c1f94568d36cdb738/src/applications/financial-status-report/sass/financial-status-report.scss#L268

@josephrlee josephrlee added bug Something isn't working platform-design-system-team labels Mar 29, 2024
@caw310
Copy link
Contributor

caw310 commented Apr 1, 2024

@jamigibbs
Copy link
Contributor

jamigibbs commented Apr 2, 2024

Can the designers weigh in on this to make sure we want to add this functionality to va-select? This would be the same as what we added here for va-text-input.

We added it to text input because it's a USWDS feature for text input but I don't see the same thing for select:

Screenshot 2024-04-02 at 9 36 26 AM

cc @danbrady @LWWright7 @babsdenney @humancompanion-usds

@LWWright7
Copy link

LWWright7 commented Apr 2, 2024

@jamigibbs - I don't see any issue with creating the same breakpoints for va-select that we use for va-text-input. Of course, the dev would need to make sure that the right size selected would correspond to the length of the selectable items in the list so the text don't get cut off. I would like to hear more input from the rest of the design team. @danbrady @babsdenney

@babsdenney
Copy link

@LWWright7 I agree with you. It doesn't seem like an issue, the main concern would be making sure the options in the select are not cut off.

@jamigibbs It is strange that the width setting is noticeably missing from the select in the USWDS. Looking at this PR, the USWDS recently updated some of their components to include new values for variants. It looks like maybe the USWDS decided not to add the width value to the Select.

USWDS-Site: Fix component pages from audit (Phase 3)

It might be worthwhile to ask the USWDS why they don't have a width variant for the select. The USWDS is manually applying a width to the examples on the select page. I would like to know more of the reasoning behind why the USWDS doesn't have a select width setting.

@jamigibbs jamigibbs added va-select DS select component and removed bug Something isn't working labels Apr 2, 2024
@jamigibbs
Copy link
Contributor

jamigibbs commented Apr 2, 2024

@josephrlee Can you provide a screenshot of the form that is using the va-select component where you'd need/want to adjust the width?

@danbrady
Copy link
Contributor

danbrady commented Apr 2, 2024

I agree with what everyone has said and don't see an issue with adding sizes. Some other random thoughts:

  • Since our components render browser-native select elements, I believe the browser/OS options popup should always be fully visible? (On mobile I think they popup in a scrollable area on the bottom of the screen?). Is this right? In other words, setting width only affects the select element, and not the popup after a user interacts with it. But this only applies to single select elements. Selects with multiple attribute could cut off the labels if the width is narrower.
  • I don't know if there's a way in code to prevent a select from being narrower than its selected option. We'll likely still need to add something in guidance about having appropriate widths to prevent option labels getting cut off in it's "selected" state. For example, "Width of select should never be shorter than the longest label".
  • The USWDS select uses usa-input as a dependency. So you could use .usa-input--[size] (which sizes text inputs) on the select element to set it's width. They also don't have success/error states for their select. (Again, a user could use .usa-input--success or .usa-input--error for this, but I'm not sure if they should.) I'm curious if the USWDS simply hasn't considered adding these classes directly to their select or maybe just didn't document to use the usa-input ones. 🤔

@josephrlee
Copy link
Contributor Author

josephrlee commented Apr 2, 2024

@jamigibbs, it's for the the type of work question on the FSR; it's in two places. One for the veteran employment question and the other for the spouse employment question:

Screenshot 2024-04-02 at 4 17 08 PM

When our custom class is not applied, it inherits the full max-width of 480px from the component:
Screenshot 2024-04-02 at 4 17 53 PM

@josephrlee
Copy link
Contributor Author

josephrlee commented Apr 2, 2024

Unrelated to this request, I think our form is probably using the wrong component altogether. There are only 4 options in this screenshot above, so according to the USWDS guidance, it should be a radio button group instead.

Screenshot 2024-04-02 at 4 23 40 PM

@jamigibbs
Copy link
Contributor

Unrelated to this request, I think our form is probably using the wrong component altogether. There are only 4 options in this screenshot above, so according to the USWDS guidance, it should be a radio button group instead.

Screenshot 2024-04-02 at 4 23 40 PM

@josephrlee That's a good find. It does look like the design system would like it to be radio group with only 4 options.

I think we can still add the width option to va-select but you might want to check in with your designer to see if you can just change your implementation to checkbox instead.

@josephrlee
Copy link
Contributor Author

@jamigibbs , I am one of the designers on the team. :) I created a ticket on our teams board to get it done. Just need to check with our FE's on the level of effort for it! Also, thanks for getting the width prop added to the va-select!

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

No branches or pull requests

6 participants