-
-
Notifications
You must be signed in to change notification settings - Fork 79k
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 form validation tooltip alignment #31557
Conversation
Thanks for the diagnostis and PR! I read your explanations carefully and tried another way: using Could you try, please—and if you can confirm this, update your PR? |
Thanks ffoodd for reviewing so quickly! 😃 I believe Snippet to reproduce in IE(taken from the Bootstrap 4.0 docs) <form class="needs-validation" novalidate>
<div class="form-row">
<div class="col-md-4 mb-3">
<label for="validationTooltip01">First name</label>
<input type="text" class="form-control" id="validationTooltip01" placeholder="First name" value="Mark" required>
<div class="valid-tooltip">
Looks good!
</div>
</div>
<div class="col-md-4 mb-3">
<label for="validationTooltip02">Last name</label>
<input type="text" class="form-control" id="validationTooltip02" placeholder="Last name" value="Otto" required>
<div class="valid-tooltip">
Looks good!
</div>
</div>
<div class="col-md-4 mb-3">
<label for="validationTooltipUsername">Username</label>
<div class="input-group">
<div class="input-group-prepend">
<span class="input-group-text" id="validationTooltipUsernamePrepend">@</span>
</div>
<input type="text" class="form-control" id="validationTooltipUsername" placeholder="Username" aria-describedby="validationTooltipUsernamePrepend" required>
<div class="invalid-tooltip">
Please choose a unique and valid username.
</div>
</div>
</div>
</div>
<div class="form-row">
<div class="col-md-6 mb-3">
<label for="validationTooltip03">City</label>
<input type="text" class="form-control" id="validationTooltip03" placeholder="City" required>
<div class="invalid-tooltip">
Please provide a valid city.
</div>
</div>
<div class="col-md-3 mb-3">
<label for="validationTooltip04">State</label>
<input type="text" class="form-control" id="validationTooltip04" placeholder="State" required>
<div class="invalid-tooltip">
Please provide a valid state.
</div>
</div>
<div class="col-md-3 mb-3">
<label for="validationTooltip05">Zip</label>
<input type="text" class="form-control" id="validationTooltip05" placeholder="Zip" required>
<div class="invalid-tooltip">
Please provide a valid zip.
</div>
</div>
</div>
<button class="btn btn-primary" type="submit">Submit form</button>
</form> |
Aww you're right, thanks -_- So back to your suggestion: we usually try to avoid CSS hacks, this is why we simply added Wouldn't your suggestion work if you simply used Any opinion, @twbs/css-review? |
Yep that should work in almost all cases, it's just in the cases where the tooltips are added to some other element with different amounts of padding that it might be misaligned - this would happen in IE, but thought a CSS hack could restrict that to IE and have other browsers display normally. Don't mind either way, happy to make it the behaviour in all browsers - as I said, in probably almost all cases it would work (and either way would be better than what we have now) |
Hi @ffoodd, sorry to bother you again - has some kind of consensus been reached about this yet? :) |
No one stepped in yet, however I'd recommend to drop the hacks since use them. |
Agreed on hacks—we don't use CSS hacks in general unless absolutely necessary. |
Sorry for the delay, should be updated now @ffoodd :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @mdo @MartijnCuppens any comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with this! To confirm, we don't need to do this in v5, correct? Could be nice to make a note of that somewhere (perhaps just in the blog post?).
BTW shouldn't we move the button a little lower so that the tooltip doesn't overlap with it? Just thinking out loud, probably affects both branches. |
@ffoodd @MartijnCuppens should we move the button a little lower with margin top? This affects v5 too IIRC. |
I don't think so:
I'd say no, but that's highly debatable. |
I personally find it very annoying that the validation feedback overlaps but maybe it's just me. :) |
IMHO the problem is inherent to absolutely-positionned things above a fine tuned layout: you'd never have room for it. I personnaly recommend to not use tooltips, so… :D |
Fixes #31507, as per my comment there (#31507 (comment))
Summary: IE is non-standards compliant about positioning. Fix was put in place which broke alignment in all browsers. This implements a more robust fix, and scopes it to IE only.
Tested in Google Chrome, Firefox, Edge, Internet Explorer 11, Google Chrome (Android).
v4-dev: https://v4-dev--twbs-bootstrap.netlify.app/docs/4.5/components/forms/#tooltips
this branch: https://deploy-preview-31557--twbs-bootstrap.netlify.app/docs/4.5/components/forms/#tooltips