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 form-control-feedback position when label has sr-only class #13320

Merged
merged 3 commits into from
May 1, 2014

Conversation

reywood
Copy link
Contributor

@reywood reywood commented Apr 10, 2014

Fixes feedback positioning in the case where the label contained within a form group has the sr-only class. Example HTML:

<div class="form-group has-feedback">
    <label class="control-label sr-only">Label</label>
    <input type="text" class="form-control">
    <span class="glyphicon glyphicon-ok form-control-feedback"></span>
</div>

This jsbin has a few form-group variations, the first of which illustrates the bug. This jsbin uses the same HTML but adds my proposed CSS fix.

This case was mentioned in issue #12873.

@cvrebert cvrebert added the css label Apr 11, 2014
@mdo
Copy link
Member

mdo commented Apr 11, 2014

Hmm, alright, I'm more partial to this now after seeing it a few times over. Thanks for opening this PR. Few points of feedback:

  • Check your indentation—Bootstrap's CSS is two spaces.
  • Can you take a stab at adding an example to our documentation with this use case?

I can help with the latter, but if you want to take a stab, I'd like to give you the chance :).

@reywood
Copy link
Contributor Author

reywood commented Apr 11, 2014

Whoops, indent fixed. I'll give it a go with the docs.

@cvrebert cvrebert added this to the v3.2.0 milestone Apr 11, 2014
@reywood
Copy link
Contributor Author

reywood commented Apr 11, 2014

Alright, I added an example to the docs and reworked the bs-callout for optional icons to match. I added H4s between the examples to break them up visually. I won't be offended if you change them. :)

@reywood
Copy link
Contributor Author

reywood commented Apr 11, 2014

One note, I did not commit the recompiled versions of bootstrap.css, etc as I figured that would make merging more of a pain.

@mdo mdo mentioned this pull request May 1, 2014
@mdo mdo merged commit 458cb69 into twbs:master May 1, 2014
@mdo
Copy link
Member

mdo commented May 1, 2014

Thanks, @reywood. Made a few changes and merged it all in. <3

@reywood
Copy link
Contributor Author

reywood commented May 1, 2014

Awesome, thanks!

@twbs twbs locked and limited conversation to collaborators Jun 14, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants