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

Add tooltip to search #2758

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions app/assets/builds/administrate/application.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion app/assets/builds/administrate/application.css.map

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions app/assets/builds/administrate/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -22172,6 +22172,14 @@
// app/assets/javascripts/administrate/controllers/index.js
application.register("select", select_controller_default);
application.register("table", table_controller_default);
var searchPopover = document.querySelector("[popover][id='search-tooltip']");
var searchTooltip = document.querySelector("button[popovertarget='search-tooltip']");
searchTooltip.addEventListener("mouseenter", () => {
searchPopover.showPopover();
});
searchTooltip.addEventListener("mouseleave", () => {
searchPopover.hidePopover();
});
})();
/*! Bundled license information:

Expand Down
4 changes: 2 additions & 2 deletions app/assets/builds/administrate/application.js.map

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions app/assets/javascripts/administrate/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,13 @@ import TableController from "./table_controller";

application.register("select", SelectController);
application.register("table", TableController);

// Bug: Visit another page and come back to this page, the mouesover popover will not work
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure why this doesn't work after navigating to another page and back. Is it something to do with Turbo page loads?

const searchPopover = document.querySelector("[popover][id='search-tooltip']");
const searchTooltip = document.querySelector("button[popovertarget='search-tooltip']");
searchTooltip.addEventListener("mouseenter", () => {
searchPopover.showPopover();
});
searchTooltip.addEventListener("mouseleave", () => {
searchPopover.hidePopover();
});
12 changes: 12 additions & 0 deletions app/assets/stylesheets/administrate/components/_buttons.scss
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,15 @@ button,
.button--nav {
margin-bottom: $base-spacing;
}

.button--tooltip {
background: none;
border: none;
color: inherit;
cursor: pointer;
padding: 0;

&:hover {
background-color: unset !important;
Copy link
Author

@jared-thoughtbot jared-thoughtbot Jan 31, 2025

Choose a reason for hiding this comment

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

The linter doesn't like !important here. This whole class is used to effectively unset all the default styles on buttons because a popover needs to use a button from what I understand

}
}
35 changes: 35 additions & 0 deletions app/assets/stylesheets/administrate/components/_search.scss
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,38 @@ $search-icon-size: 1rem;
fill: $action-color;
}
}

.search__tooltip {
anchor-name: --tooltip-anchor;
margin-right: 2rem;

svg {
fill: $grey-5;
height: 24px;
width: 24px;

&:hover {
fill: $action-color;
}
}
}

.search__tooltip-popover {
left: anchor(center);
margin: 1rem;
position: fixed;
position-anchor: --tooltip-anchor;
top: anchor(bottom);
transform: translateX(-50%);

background-color: $blue;
border-color: $blue;
border-radius: $base-border-radius;
color: $white;
padding: 2rem;
width: max-content;
Comment on lines +64 to +76
Copy link
Author

Choose a reason for hiding this comment

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

The first set of styles is all to do with positioning the popover, the second set (after the gap) is all to do with the visual style of the popover. The linter doesn't like the gap and the non alphabetical between them. It feels useful to separate the styles as I have. Shall I created 2 separate classes?

}

.search__tooltip-popover-value {
opacity: 0.5;
}
4 changes: 3 additions & 1 deletion app/controllers/administrate/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ def index
resources = order.apply(resources)
resources = paginate_resources(resources)
page = Administrate::Page::Collection.new(dashboard, order: order)
filters = Administrate::Search.new(scoped_resource, dashboard, search_term).valid_filters

render locals: {
resources: resources,
search_term: search_term,
page: page,
show_search_bar: show_search_bar?
show_search_bar: show_search_bar?,
filters: filters
Copy link
Author

Choose a reason for hiding this comment

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

Do we want an option to disable the tooltip even if there are collection
filters?

}
end

Expand Down
4 changes: 4 additions & 0 deletions app/views/administrate/application/_icons.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@
<symbol id="icon-up-caret" viewBox="0 0 48 48">
<path d="M2.988 33.02c-1.66 0-1.943-.81-.618-1.824l20-15.28c.878-.672 2.31-.67 3.188 0l20.075 15.288c1.316 1.003 1.048 1.816-.62 1.816H2.987z" />
</symbol>

<symbol id="icon-question-mark" viewBox="0 0 24 24">
<path fill-rule="evenodd" d="M2.25 12c0-5.385 4.365-9.75 9.75-9.75s9.75 4.365 9.75 9.75-4.365 9.75-9.75 9.75S2.25 17.385 2.25 12Zm11.378-3.917c-.89-.777-2.366-.777-3.255 0a.75.75 0 0 1-.988-1.129c1.454-1.272 3.776-1.272 5.23 0 1.513 1.324 1.513 3.518 0 4.842a3.75 3.75 0 0 1-.837.552c-.676.328-1.028.774-1.028 1.152v.75a.75.75 0 0 1-1.5 0v-.75c0-1.279 1.06-2.107 1.875-2.502.182-.088.351-.199.503-.331.83-.727.83-1.857 0-2.584ZM12 18a.75.75 0 1 0 0-1.5.75.75 0 0 0 0 1.5Z" clip-rule="evenodd" />
</symbol>
</svg>
27 changes: 27 additions & 0 deletions app/views/administrate/application/_index_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@
<%= display_resource_name(page.resource_name) %>
<% end %>

<%#
TODO: Move where this polyfill lives. Where?
Also, do we want to import the polyfill js directly instead of linking to unpkg.com?
%>
<script type="module">
if (!("anchorName" in document.documentElement.style)) {
import("https://unpkg.com/@oddbird/css-anchor-positioning");
}
Comment on lines +6 to +12
Copy link
Author

Choose a reason for hiding this comment

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

  • Where should this polyfill live?
  • Do we want to import the polyfill js directly instead of linking to unpkg.com?

</script>

<header class="main-content__header">
<h1 class="main-content__page-title" id="page-title">
<%= content_for(:title) %>
Expand All @@ -15,6 +25,23 @@
search_term: search_term,
resource_name: display_resource_name(page.resource_name)
) %>

<% if filters.any? %>
<button popovertarget="search-tooltip" class="button--tooltip search__tooltip">
<svg role="img">
<use xlink:href="#icon-question-mark" />
</svg>
</button>

<div popover id="search-tooltip" role="tooltip" class="search__tooltip-popover">
<p><strong>Use filters to refine your search</strong></p>
<ul>
<% filters.keys.each do |filter_key| %>
<li><%= filter_key %>:<span class="search__tooltip-popover-value">&lt;value&gt;</span></li>
<% end %>
</ul>
</div>
<% end %>
<% end %>

<div>
Expand Down
1 change: 1 addition & 0 deletions app/views/administrate/application/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ It renders the `_table` partial to display details about the resources.
search_term: search_term,
page: page,
show_search_bar: show_search_bar,
filters: filters,
)
%>

Expand Down
16 changes: 8 additions & 8 deletions lib/administrate/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ def run
end
end

def valid_filters
if @dashboard.class.const_defined?(:COLLECTION_FILTERS)
@dashboard.class.const_get(:COLLECTION_FILTERS).stringify_keys
else
{}
end
end

private

def apply_filter(filter, filter_param, resources)
Expand Down Expand Up @@ -116,14 +124,6 @@ def search_results(resources)
.where(query_template, *query_values)
end

def valid_filters
if @dashboard.class.const_defined?(:COLLECTION_FILTERS)
@dashboard.class.const_get(:COLLECTION_FILTERS).stringify_keys
else
{}
end
end

def attribute_types
@dashboard.class.const_get(:ATTRIBUTE_TYPES)
end
Expand Down
48 changes: 48 additions & 0 deletions spec/features/search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,54 @@ def have_a_search_bar
end
end

describe "search bar tooltip" do
def have_a_search_tooltip
have_css("button[popovertarget=search-tooltip]")
end

def search_tooltip_icon
find("button[popovertarget=search-tooltip]")
end

it "is visible when the current dashboard has collection filters" do
visit admin_customers_path

expect(page).to have_a_search_tooltip
end

context "when clicked" do
it "shows a popover with the available filters" do
visit admin_customers_path

search_tooltip_icon.click

expect(page).to have_content("Use filters to refine your search")
expect(page).to have_content("vip:<value>")
expect(page).to have_content("kind:<value>")
end
end

it "is hidden when the current dashboard has no collection filters" do
stub_const("CustomerDashboard::COLLECTION_FILTERS", {})

visit admin_customers_path

expect(page).not_to have_a_search_tooltip
end

it "is hidden when nothing is searchable in the current dashboard" do
CustomerDashboard::ATTRIBUTE_TYPES.each do |_name, field_class|
allow(field_class).to(
receive(:searchable?).and_return(false)
)
end

visit admin_customers_path

expect(page).not_to have_a_search_tooltip
end
end

scenario "admin searches for customer by email", :js do
query = "[email protected]"
perfect_match = create(:customer, email: "[email protected]")
Expand Down
Loading