salcode / bootstrap-genesis

WordPress Genesis Child Theme setup to use Bootstrap, Sass, and Grunt
MIT License
184 stars 63 forks source link

Adding bootstrap styled search revisited #109

Closed bryanwillis closed 8 years ago

bryanwillis commented 8 years ago

This has to do with the search form added here

Been meaning to get back to this for awhile, but kept forgetting about it. The 1% width .input-group-button isn't getting priority over the auto width in the media query so it's not being displayed right.

So it's looking like this:

screen shot 2015-10-27 at 5 48 25 pm

Instead of being full-width on all displays like this:

screen shot 2015-10-27 at 5 49 54 pm

There are are a lot of ways to fix this.

1) Just target 768px as it seems to be only one affected:

@media (min-width: 768px) {
.search-form.form-inline .input-group > .input-group-btn {
    width: 1%;
 }
}

2) Use the sibling selector which will get priority:

input#bsg-search-form ~ .input-group-btn {
     width: 1%;
}

3) Use !important (hacky solution):

.search-form .input-group-btn {
     width: 1% !important;
}

On a side note from the search bar width, we also might still need to move the glyphicon search icon down another pixel, at least the way I'm seeing it. This could possibly be retina mac display only though.

.search-form .glyphicon {
    top: 2px;
}

Alternatively, we could fix both of these issues using flexbox. While the code here is the longest it is also way cleaner than bootstraps 1% width hack.

.search-form.form-inline .input-group, 
.search-form.form-inline .input-group .input-group-btn {
    display: flex;
}
#bsg-search-form {
    flex: 1;
}
button.search-submit.btn.btn-default {
    display: inline-flex;
    padding: 0 14px;
    justify-content: center;  /* might not be needed */
    align-items: center; /*  might not be needed  */
}
@media (min-width: 768px) {
   .search-form.form-inline > .form-group {
        display: block;
    }
}
salcode commented 8 years ago

@bryanwillis If I understand you correctly it sounds like this is a problem at the Bootstrap level, is that correct?

If so, I'm not sure if we want to put the fix in on this project or if we want to bring it up on the Bootstrap project.

My original goal with this theme was to include no original CSS and do all styling by manipulating the markup to work with Bootstrap. My thinking here was this should make updating the Bootstrap component as easy as possible. It has been necessary to add some non-Bootstrap CSS to handle things like Widgets and the Genesis Pagination markup not matching the Bootstrap markup 100%. That said, I still like the idea of avoiding adding CSS unless absolutely necessary.

Are there improvements we can make to the markup to help these areas render better or do you think these need to be CSS fixes?

salcode commented 8 years ago

I'm open to changing markup but I'd like to let the Bootstrap CSS style this form.

I'm not familiar with a markup change that will improve this rendering but if someone has a suggestion, feel free to re-open this issue.