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

Surround searchbox inputs with a div for styling #3616

Merged
merged 4 commits into from May 6, 2024

Conversation

maccabeelevine
Copy link
Member

@maccabeelevine maccabeelevine commented Apr 24, 2024

This is essentially a two-line PR that encloses the searchbox inputs with a named div so that they can be styled. For example the line of inputs could be a flexbox or grid layout.

image

Currently those elements share the same parent element <form id="searchForm" with the search tabs (among other stuff), so they are difficult to style.

Comment on lines 1 to 18
<style>
.searchForm-inputs {
display: flex;
flex-wrap: wrap;
gap: 0.5rem;
}

.searchForm-inputs .searchForm-query {
width: auto;
flex-grow: 1;
}

@media screen and (min-width: 768px) {
.searchForm-inputs .searchForm-query {
flex-grow: 0;
}
}
</style>
Copy link
Member Author

Choose a reason for hiding this comment

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

These styles are just an example and would be removed before merge. The effect is to let the main search box grow on mobile widths to take up whatever space it has available, but to still fit the Find button on the same line if there is nothing else in between. It's nice space saver.

echo '<input type="hidden" name="sort" value="' . $this->escapeHtmlAttr($lastSort) . '">';
}
?>
</div>
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not actually sure how much should be included within the div. I think everything within it here is inline, and that the filters below are supposed to be below.

@maccabeelevine maccabeelevine marked this pull request as ready for review April 24, 2024 19:06
@demiankatz
Copy link
Member

Thanks, @maccabeelevine -- I'll defer to @crhallberg on this one (though would also welcome opinions from others). Chris is out of office today and might need a little time to catch up after he returns tomorrow, but hopefully you'll hear from him fairly soon. :-)

Copy link
Contributor

@crhallberg crhallberg left a comment

Choose a reason for hiding this comment

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

This looks good! Don't forget to clear out the example styles!

@maccabeelevine
Copy link
Member Author

This looks good! Don't forget to clear out the example styles!

Thanks. I've removed the example CSS.

@sturkel89
Copy link
Contributor

I reviewed the test branch in all three themes by adding filters and resizing the browser window from widest to narrowest. I compared behavior between test and dev in each of those environments.

Everything looked identical to me between test and dev in each of the themes. Unless I'm missing something, I would say that this is ready to merge.

@demiankatz demiankatz merged commit 4a60b1c into vufind-org:dev May 6, 2024
7 checks passed
@demiankatz
Copy link
Member

Thanks, everyone!

@demiankatz demiankatz added this to the 10.0 milestone May 6, 2024
@maccabeelevine maccabeelevine deleted the searchform-inputs-div branch May 6, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants