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

Handle overflow in SelectNext #1325

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Handle overflow in SelectNext #1325

merged 1 commit into from
Oct 17, 2023

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Oct 16, 2023

Closes #1249

This PR is trying to make SelectNext behave closer to the native select when the items overflow the size of the select itself.

For context, this is how a native select looks in that case:

Captura desde 2023-10-16 15-33-07

  • The content in the select itself is cropped, to make sure it does not overflow its container.
  • The listbox grows to fit all options without cropping them. This is important, as there has to be a place where you can see the full content for every option.

Taking that into consideration, the next changes have been made.

  1. The listbox has changed from w-full to min-w-full. That way it keeps looking the same as before in most of the cases, but it grows if the content is too long.

    image
    image

  2. The toggle button now truncates its content. That ensures there won't be content overflow, and displays ellipsis if the content is just plain text.
    When the content is not plain text, consumers might need to adapt their code in order to make it look nice. An example of this is included in the pattern library docs.

    image

Testing steps.

You can check the new examples at the bottom of http://localhost:4001/input-select-next

@acelaya acelaya requested a review from robertknight October 16, 2023 15:45
@acelaya acelaya force-pushed the select-next-overflow branch from 0f1aa11 to dedf468 Compare October 16, 2023 16:05
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #1325 (0e8e03c) into main (de1df29) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main     #1325   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           62        62           
  Lines          921       921           
  Branches       352       352           
=========================================
  Hits           921       921           
Files Coverage Δ
src/components/input/SelectNext.tsx 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@acelaya acelaya force-pushed the select-next-overflow branch from dedf468 to 0e8e03c Compare October 17, 2023 07:47
@acelaya
Copy link
Contributor Author

acelaya commented Oct 17, 2023

@robertknight I have pushed the next changes.

  • I have tried to improve the explanation of the component's behavior when content would overflow, trying to clarify the behaviors of the toggle button vs the listbox.
  • I have fixed the miss-behavior you detected, where the button size changed depending on selected option, rather than hiding the overflowing content.
  • I have added one more example where an InputGroup is used.

Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

LGTM

@acelaya acelaya merged commit 132ca0d into main Oct 17, 2023
@acelaya acelaya deleted the select-next-overflow branch October 17, 2023 15:37
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.

SelectNext: Handle too long content in options or select
2 participants