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

Search Bar Display Logic #730

Merged

Conversation

@akestner
Copy link
Contributor

akestner commented Jan 13, 2017

Currently the search bar is shown regardless of whether or not a resource contains searchable attributes. This PR adds a method to the Administrate::ApplicationController that checks whether any attributes are searchable and passes a new local variable (show_search_bar) to the view which is then used to determine whether or not to render the search partial.

akestner added 3 commits Jan 13, 2017
* extracts capture_view_locals method to ControllerHelpers
end

show_search_bar
end

This comment has been minimized.

Copy link
@BenMorganIO

BenMorganIO Jan 13, 2017

Collaborator

Refactor suggestion:

# Checks if any of the dashboard's collection attributes are searchable. If none
# of them are, then we know that there isn't any need to load the search bar. In
# which case, we hide it.
#
# @return [Boolean]
def show_search_bar?
  dashboard.attribute_types_for(
    dashboard.collection_attributes
  ).any? do |_name, attribute|
    attribute.searchable?
  end
end

This comment has been minimized.

Copy link
@akestner

akestner Jan 13, 2017

Author Contributor

Nice, I'll update.

@BenMorganIO

This comment has been minimized.

Copy link
Collaborator

BenMorganIO commented Jan 13, 2017

I really like this PR. Left some comments. Great work @akestner 👍

@akestner

This comment has been minimized.

Copy link
Contributor Author

akestner commented Jan 13, 2017

Thanks @BenMorganIO! I initially wrote it for a client and thought it seemed like a good change to submit back upstream.

I've updated the show_search_bar? method to make use of .any? like you suggested, though I left out the documentation as I didn't see many other documented methods in the codebase.

@BenMorganIO BenMorganIO merged commit b28f125 into thoughtbot:master Jan 17, 2017
1 check passed
1 check passed
hound No violations found. Woof!
fwolfst added a commit to fwolfst/administrate that referenced this pull request Mar 8, 2017
* hides search bar when no attributes are searchable

* adds tests for search bar display logic
* extracts capture_view_locals method to ControllerHelpers

* no need for page argument to show_search_bar? method

* streamlines show_search_bar? method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.