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 floating labels with form-select sizes #41013

Merged

Conversation

patrickzzz
Copy link
Contributor

@patrickzzz patrickzzz commented Nov 7, 2024

Description

This pull request addresses an alignment issue with Bootstrap’s floating labels when using the small form-select-sm element. The problem occurs because the selected value is misaligned with the floating label due to reduced padding applied to the small select. By adjusting the padding-left property to match the floating label's padding, this PR ensures the selected value aligns properly with the label.

Motivation & Context

The change is required to fix the visual misalignment of the form-select-sm element when used with floating labels, as reported in GitHub Issue #41008. This fix ensures a consistent and visually correct layout for the small select element when used with floating labels.

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

You can see the changes in the live preview, when adding the form-select-sm class:

Related issues

Copy link

@abay-2002 abay-2002 left a comment

Choose a reason for hiding this comment

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

I understand the intention behind this change to address the padding misalignment for form-select-sm with floating labels, but I’d like to see whether this adjustment breaks any other components, particularly for larger select boxes or other form controls. It could be useful to test this across various form elements and screen sizes to ensure consistent behavior. If this padding tweak resolves the issue without side effects, it seems like a good solution to align the selected value with the floating label.

@coliff
Copy link
Contributor

coliff commented Nov 11, 2024

Great PR. I just tested this in a StackBlitz with the CSS generated from the deploy preview.

https://stackblitz.com/edit/bzr9zn?file=index.html

Separate issue I spotted though is that text is cropped on the Large variant (see the 'p' in the word 'Open')

@patrickzzz
Copy link
Contributor Author

Hi @abay-2002 and @coliff, thanks for your feedback!

Yes, I have also observed the behavior with form-select-lg.

This PR includes an adjustment to the padding-left for both form-select-sm and form-select-lg, aiming to improve alignment for selected values across sizes.

As for the cropped text in form-select-lg, I also see this as a separate issue, as it may require a different approach.

Thanks again for your insights!

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

I'm fine with the actual changes.

Just requesting changes for https://deploy-preview-41013--twbs-bootstrap.netlify.app/docs/5.3/forms/floating-labels/#selects text to update and remove the mention of size since it works now thanks to you.

It could be useful to test this across various form elements and screen sizes to ensure consistent behavior.

It looks like a quite located change since it's managed by .form-floating > * rule.

As for the cropped text in form-select-lg, I also see this as a separate issue, as it may require a different approach.

It may be related to #39483 indeed.

@patrickzzz
Copy link
Contributor Author

patrickzzz commented Nov 13, 2024

Hi @louismaximepiton,

Thanks for the feedback!

The documentation at this point refers to the select’s size attribute (https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/size), which is still not fully supported by Bootstraps .form-floating. My changes only address alignment issues in form-select-sm and form-select-lg variants with floating labels, but they don’t affect or enable the size attribute functionality. So we’ll need to keep that note in the docs.

Otherwise, I completely agree with your other points!

Edit: Or do you mean especially this sentence Other than .form-control, floating labels are only available on .form-selects.?

Regards,
Patrick

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

Nvm, you're right, I missed that 🤐
The PR looks good, thanks again for the contribution!

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @patrickzzz and all for the team effort to review it!
LGTM! Also think that the cropped text issue on the large variant can be tackled in another PR and/or is a different issue.
Let's do it step by step :)

Just added a test case in js/tests/visual/floating-label.html based on https://stackblitz.com/edit/bzr9zn?file=index.html. I'll add Christian to the co-authored for that too.

@julien-deramond julien-deramond merged commit cacbdc6 into twbs:main Nov 14, 2024
14 checks passed
@theodorejb
Copy link

@julien-deramond The cropped text issue would indeed be fixed by #39483. I'm still hoping it can be merged and included in the next patch release. I just rebased it again to fix the merge conflict following this PR.

@julien-deramond
Copy link
Member

Thank you, @theodorejb, I'll try to find time to review it soon.

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

Successfully merging this pull request may close these issues.

Alignment issue with floating labels and small select (form-select-sm) padding in Bootstrap 5.3
6 participants