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

One Searchbox #814

Merged
merged 12 commits into from
Nov 28, 2016
Merged

One Searchbox #814

merged 12 commits into from
Nov 28, 2016

Conversation

crhallberg
Copy link
Contributor

@crhallberg crhallberg commented Sep 29, 2016

Simplify VuFind to only have one searchbox.

All checked installations of VuFind using the searchbox in-line with the logo and menu had completely custom layouts. Having the searchbox on the next line is the most popular. This PR removes the top searchbox and simplifies css and js logic.

  • IDs or classes (discussion at VuFind)

Chris Hallberg added 9 commits September 16, 2016 14:57
@demiankatz
Copy link
Member

I believe that, in our discussion of this, it was suggested that we should continue to use a class instead of an ID for the search box even if we remove the duplicate one. (I think there were some use cases around embedding multiple VuFind boxes on a single page, though I can't remember who mentioned this or what the details were). In any case, reverting the class-->id switch would certainly make the diffs in this PR smaller. What do you think?

@crhallberg crhallberg mentioned this pull request Oct 14, 2016
11 tasks
@demiankatz
Copy link
Member

@crhallberg, are there any outstanding issues here? This seems to be working fine for me. If you're not aware of any remaining issues, perhaps you should run the full test suite on this branch one more time to be sure all is well and then merge if so.

@crhallberg
Copy link
Contributor Author

Some errors are coming up in the full test, maybe be a cascading timing issue though. Working on it now.

@crhallberg
Copy link
Contributor Author

Finally got my test instance working consistently again. Tests have passed. Sorry for the delay.

@crhallberg crhallberg self-assigned this Nov 22, 2016
@demiankatz demiankatz merged commit 3619aa0 into vufind-org:master Nov 28, 2016
@demiankatz
Copy link
Member

Thanks, Chris, I've gone ahead and merged this.

@crhallberg crhallberg deleted the one-search branch June 26, 2017 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants