-
Notifications
You must be signed in to change notification settings - Fork 10
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
Search bar layout #59
Conversation
@@ -0,0 +1,3 @@ | |||
<button class="btn btn-default navbar-btn disabled"> | |||
catalog <span class="caret"></span> | |||
</button> |
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.
So I think we'll end up shipping this w/ only "Catalog" search at first and the word catalog is establishing the affordance for what will end up being more options. We may want to not have any indication that this is a physical button at the moment, but I'll let @jvine chime in on if shipping this as a physical button at this point makes sense.
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.
Let's leave is as a button for now; I think it will be useful to get feedback on the concept. Can remove the carat later if necessary.
This needs a rebase @mejackreed |
I think there are some questions around the placement of the |
@jkeck also rebased |
@mejackreed I just noticed this - the search box should not have placeholder text. In the mockups, the search box only has text in it when there are actual search terms - that's intentional. |
@mejackreed since we're going the route of rendering the search form several times in this PR can we make sure we talk over our other options first. I would really like to make sure it's our only option if we decide to ship this. |
@jkeck Definitely... It doesn't feel elegant here and there is a JavaScript solution available instead |
@mejackreed okay, so while not perfect, this is kind of what I'm thinking: search-bar...search-bar-idea You can see the resulting partial https://github.com/sul-dlss/SearchWorks/blob/45a28fea155ba15db17cc9c89908b633d092e01f/app/views/shared/_search_bar.html.erb |
adds navlist partial and responsive tests
This pull request fixes #2
I've gone through several iterations to get the functionality and responsiveness working correctly. Unfortunately I'm still not complete but the lacking responsive view is the one that we are still waiting to hear back on design. Will keep working on this, but I wanted to put it up to hopefully get some feedback.