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

Fix horizontal overflow for selects with long names in sidebar #3786

Merged
merged 2 commits into from
Dec 4, 2017

Conversation

laras126
Copy link
Contributor

@laras126 laras126 commented Dec 3, 2017

Description

Fix for #3518.
The select element was not responding to Flexbox appropriately in Webkit browsers.

How Has This Been Tested?

Device: MacBook Pro 2015
Browsers: Chrome 62.0.3202.94, Safari 11.0.1, FireFox 57.0.1

Screenshots (jpeg or gifs if applicable):

sidebar-author-select

Types of changes

Adding a min-width: 0; to select elements solves this issue.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@laras126 laras126 changed the title Use min-width to make select respond to Flexbox Fix horizontal overflow for selects with long names in sidebar Dec 3, 2017
@ellatrix ellatrix requested a review from aduth December 3, 2017 20:58
Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

Tested and looks great, especially after the last change that makes sure the label does not shrink at smaller widths.

@laras126
Copy link
Contributor Author

laras126 commented Dec 3, 2017

To expand on the last commit for the label -
label-fix
Note that the "After" screenshot above shows the label for "Stick to the front page" wrapping, but I modified the max-width in my last commit to be a bit larger so this does not happen.

@laras126 laras126 force-pushed the fix/fix-select-overflow branch from b41ec3c to aab9dcc Compare December 3, 2017 22:04
@youknowriad youknowriad merged commit 7efa54c into WordPress:master Dec 4, 2017
label {
margin-right: 10px;
flex-shrink: 0;
max-width: 75%;
Copy link
Member

Choose a reason for hiding this comment

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

Was this necessary?

Copy link
Contributor Author

@laras126 laras126 Dec 4, 2017

Choose a reason for hiding this comment

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

Not necessary for the fix itself, but it balances out the label width to keep it from wrapping when the select is long and label short - see this comparison:

label-fix

Note that the "After" screenshot above shows the label for "Stick to the front page" wrapping, but the max-width at 75% instead solves that.

Copy link
Member

Choose a reason for hiding this comment

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

Would this have been possible to achieve with white-space: nowrap?

https://developer.mozilla.org/en-US/docs/Web/CSS/white-space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For short label like this one, yes, but we wouldn't want to prevent wrapping for very long labels – with this solution, the label will wrap only when it starts to fill more than the max-width of 75%.

@afercia
Copy link
Contributor

afercia commented Dec 17, 2024

Noting that the max-width: 75%; may not work well with longer strings for checkboxes and radio buttons. See for example the screenshot below from the entities saved panel:

Screenshot 2024-12-17 at 11 08 02

At the very leaat checkboxes and radio buttons should be excluded, however 75% seems more a magic nujber to me and ideally a more solid solution should be explored. Discovered on #67792 (comment)

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.

5 participants