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

Align uui-select with other inputs #658

Merged
merged 8 commits into from
Nov 16, 2023

Conversation

iOvergaard
Copy link
Contributor

@iOvergaard iOvergaard commented Nov 16, 2023

Description

Based on this comment we have moved a few fixes back to this library: umbraco/Umbraco-CMS#15037 (comment)

This aligns uui-select and uui-input with each other, so they work well together in a form having the same height and background-color properties.

We also now document the custom CSS properties in both elements and the story for a combined form has been fixed.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

@iOvergaard iOvergaard added bug Something isn't working documentation Improvements or additions to documentation labels Nov 16, 2023
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-beach-055ecb503-658.westeurope.azurestaticapps.net

Copy link
Contributor

@loivsen loivsen left a comment

Choose a reason for hiding this comment

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

image
Seems to be a small placement difference between the two when they are next to each other. Text alignment seems fine, but it looks like the border of select is 2 pixels off.

…because it is easiest to control the select rather than mimick it in terms of :focus and :hover on the outer element
@iOvergaard
Copy link
Contributor Author

@loivsen good catch. I moved the height to the inner element where the other properties are so they at least on paper have the same 33px default height.

(If you inspect the elements, you can check it)

The browser still places the elements a bit out of line, however if you add display: flex or some other kind of placement CSS to a wrapper div, they should be perfectly aligned like this:

image

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-beach-055ecb503-658.westeurope.azurestaticapps.net

@loivsen
Copy link
Contributor

loivsen commented Nov 16, 2023

@loivsen good catch. I moved the height to the inner element where the other properties are so they at least on paper have the same 33px default height.

(If you inspect the elements, you can check it)

The browser still places the elements a bit out of line, however if you add display: flex or some other kind of placement CSS to a wrapper div, they should be perfectly aligned like this:

image

Alternative would be to adjust the padding in the select element to have 2 pixels more in the top than in the bottom, but it's probably best if we avoid too much magic numbers.
I'll approve this

@iOvergaard iOvergaard merged commit b69ed74 into v1/contrib Nov 16, 2023
9 of 10 checks passed
@iOvergaard iOvergaard deleted the v1/feature/align-select-with-others branch November 16, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants