Skip to content

Commit d8f523b

Browse files
committed
Refactor combobox for more flexible rendering
* Uses a hidden :name field to generate dom ids * Allows for listbox option rendering to be customizable by result
1 parent be3f888 commit d8f523b

File tree

9 files changed

+177
-47
lines changed

9 files changed

+177
-47
lines changed

app/controllers/searches_controller.rb

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,37 +2,47 @@
22

33
class SearchesController < ApplicationController
44
rescue_from Searches::ParseFailed do |error|
5+
query = params.fetch(:query, "")
6+
57
respond_to do |format|
6-
format.html { render Searches::ShowView.new(pages: [], query: params[:query]) }
8+
format.html { render Searches::ShowView.new(query:, name:) }
79
format.turbo_stream {
810
render turbo_stream: [
9-
turbo_stream.replace("search-listbox", Searches::Listbox.new(pages: [], query: params[:query]))
11+
turbo_stream.replace(Searches::Listbox.dom_id(name), Searches::Listbox.new(query:, name:))
1012
]
1113
}
1214
end
1315
end
1416

1517
def show
16-
raw_query = params[:query] || ""
17-
pages = if raw_query.present?
18-
query = Searches::Query.parse!(raw_query)
19-
Page.search("#{query}*").with_snippets.ranked.limit(3)
18+
results = if query.present?
19+
parsed_query = Searches::Query.parse!(query)
20+
Page.search("#{parsed_query}*").with_snippets.ranked.limit(3)
2021
else
2122
[]
2223
end
2324

2425
respond_to do |format|
25-
format.html {
26-
render Searches::ShowView.new(pages:, query: raw_query)
27-
}
26+
format.html { render Searches::ShowView.new(results:, query:) }
27+
2828
format.turbo_stream {
2929
render turbo_stream: [
3030
turbo_stream.replace(
31-
params.fetch(:listbox_id, "search-listbox"),
32-
Searches::Listbox.new(pages: pages, query: raw_query)
31+
Searches::Listbox.dom_id(name),
32+
Searches::Listbox.new(results:, query:, name:)
3333
)
3434
]
3535
}
3636
end
3737
end
38+
39+
private
40+
41+
def query
42+
params.fetch(:query, "")
43+
end
44+
45+
def name
46+
params.fetch(:name, "search")
47+
end
3848
end

app/models/page.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,6 @@ def body_text
3131
def resource
3232
Sitepress.site.get(request_path)
3333
end
34+
35+
def url = request_path
3436
end
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
<%= render Searches::Dialog.new unless current_page?(search_path) %>
1+
<%= render Searches::Dialog.new unless current_page?(search_path) || request.post? %>

app/views/searches/combobox.rb

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ class Combobox < ApplicationComponent
44
include Phlex::Rails::Helpers::FormWith
55
include PhlexConcerns::SvgTag
66

7-
attr_reader :pages, :query
7+
attr_reader :query, :name, :attributes
88

9-
def initialize(query: "", pages: [])
10-
@pages = pages
9+
def initialize(query: "", name: "search", **attributes)
1110
@query = query
11+
@name = name
12+
@attributes = attributes
1213
end
1314

1415
def view_template
@@ -41,7 +42,7 @@ def view_template
4142
autofocus: true,
4243
role: "combobox",
4344
aria: input_aria,
44-
id: combobox_id,
45+
id: combobox_dom_id,
4546
data: {
4647
action: "
4748
focus->combobox#tryOpen
@@ -52,30 +53,30 @@ def view_template
5253
class: "w-full step-1"
5354
end
5455

55-
plain f.hidden_field :listbox_id, value: listbox_id
56+
f.hidden_field :name, value: name
5657
end
5758

58-
render Searches::Listbox.new(pages: pages, query: query)
59+
render Searches::Listbox.new(query:, name:, **attributes)
5960
end
6061
end
6162

6263
def input_aria
6364
{
6465
expanded: false,
6566
autocomplete: "none",
66-
controls: listbox_id,
67-
owns: listbox_id,
67+
controls: listbox_dom_id,
68+
owns: listbox_dom_id,
6869
haspopup: "listbox",
6970
activedescendant: ""
7071
}
7172
end
7273

73-
def combobox_id
74-
"search-combobox"
74+
def combobox_dom_id
75+
"#{name}-combobox"
7576
end
7677

77-
def listbox_id
78-
"search-listbox"
78+
def listbox_dom_id
79+
Searches::Listbox.dom_id(name)
7980
end
8081
end
8182
end

app/views/searches/listbox.rb

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,40 +2,38 @@ module Searches
22
class Listbox < ApplicationComponent
33
include Phlex::Rails::Helpers::DOMID
44

5-
attr_reader :pages, :query
5+
attr_reader :results, :query, :name
66

7-
def initialize(pages: [], query: "")
8-
@pages = pages
7+
def self.dom_id(name)
8+
"#{name}-listbox"
9+
end
10+
11+
def initialize(results: [], query: "", name: "search")
12+
@results = results
913
@query = query
14+
@name = name
1015
end
1116

1217
def view_template
1318
ul(**mix(
14-
id: "search-listbox",
19+
id: listbox_dom_id,
1520
role: "listbox",
1621
class: ["grid"],
1722
data: {
1823
controller: "search-listbox"
1924
}
2025
)) do
21-
if pages.any?
22-
pages.each.with_index do |page, i|
26+
# Using `results.present?` instead of `results.any?` is a concious decision to avoid an extra "SELECT 1" query
27+
# For more information, see: https://www.speedshop.co/2019/01/10/three-activerecord-mistakes.html#any-exists-and-present
28+
if results.present?
29+
results.each.with_index do |result, i|
2330
li(
24-
aria: {label: page.title},
31+
aria: {label: result.title},
2532
role: "option",
26-
id: dom_id(page, "search-option"),
33+
id: dom_id(result, "search-option"),
2734
class: "rounded"
2835
) do
29-
a(
30-
href: page.request_path,
31-
data: {
32-
turbo_frame: "_top"
33-
},
34-
class: ["p-2", "block"]
35-
) do
36-
div(class: "font-semibold") { raw safe(page.title_snippet) }
37-
div(class: "text-sm") { raw safe(page.body_snippet) }
38-
end
36+
render Searches::Result.new(result)
3937
end
4038
end
4139
elsif query_long_enough?
@@ -59,5 +57,9 @@ def view_template
5957
def query_long_enough?
6058
query && query.length > 2
6159
end
60+
61+
def listbox_dom_id
62+
self.class.dom_id(@name)
63+
end
6264
end
6365
end

app/views/searches/result.rb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
module Searches
2+
class Result < ApplicationComponent
3+
class_attribute :result_classes
4+
self.result_classes = {}
5+
6+
def self.register(result_class, component_class)
7+
result_classes[result_class] = component_class
8+
end
9+
10+
def self.lookup_component_class(result)
11+
component_class = result_classes[result.class]
12+
13+
if !component_class
14+
component_class = "Searches::Results::#{result.class.name}".safe_constantize
15+
register(result.class, component_class) if component_class
16+
end
17+
18+
raise ArgumentError, "[#{self}] No component registered for #{result.class}" unless component_class
19+
20+
component_class
21+
end
22+
23+
attr_reader :result, :component_class
24+
25+
def initialize(result)
26+
@result = result
27+
28+
@component_class = self.class.lookup_component_class(result)
29+
end
30+
31+
def view_template
32+
render component_class.new(result)
33+
end
34+
end
35+
end

app/views/searches/results/page.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
module Searches
2+
module Results
3+
class Page < ApplicationComponent
4+
attr_reader :page
5+
6+
def initialize(page)
7+
@page = page
8+
end
9+
10+
def view_template
11+
a(
12+
href: page.url,
13+
data: {
14+
turbo_frame: "_top"
15+
},
16+
class: ["p-2", "block"]
17+
) do
18+
div(class: "font-semibold") { raw safe(page.title_snippet) }
19+
div(class: "text-sm") { raw safe(page.body_snippet) }
20+
end
21+
end
22+
23+
Searches::Result.register(::Page, self)
24+
end
25+
end
26+
end

app/views/searches/show_view.rb

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@ class Searches::ShowView < ApplicationView
33
include Phlex::Rails::Helpers::Routes
44
include Phlex::Rails::Helpers::TurboFrameTag
55

6-
attr_reader :pages, :query
6+
attr_reader :attributes
77

8-
def initialize(pages:, query:)
9-
@pages = pages
10-
@query = query
8+
def initialize(**attrs)
9+
@attributes = attrs
1110
end
1211

1312
def view_template
@@ -16,7 +15,7 @@ def view_template
1615
class: "section-content container py-gap mb-3xl"
1716
) do
1817
h3 { "Enter your search query" }
19-
render Searches::Combobox.new(pages:, query:)
18+
render Searches::Combobox.new(**attributes)
2019
render partial: "searches/help"
2120
end
2221
end

spec/views/searches/listbox_spec.rb

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
require "rails_helper"
2+
3+
RSpec.describe Searches::Listbox, type: :view do
4+
let(:search_result) do
5+
Data.define(:title, :url, :body) do
6+
def to_key = [url.parameterize]
7+
8+
def model_name = ActiveModel::Name.new(self, nil, "SearchResult")
9+
end
10+
end
11+
12+
let(:search_result_component) do
13+
Class.new(ApplicationComponent) do
14+
attr_reader :search_result
15+
16+
def initialize(search_result)
17+
@search_result = search_result
18+
end
19+
20+
def view_template
21+
a(href: search_result.url) do
22+
div { raw safe(search_result.title) }
23+
div { raw safe(search_result.body) }
24+
end
25+
end
26+
end
27+
end
28+
29+
it "renders search results" do
30+
Searches::Result.register(search_result, search_result_component)
31+
32+
search_results = [
33+
search_result.new(title: "Joy of Rails", url: "/articles/joy-of-rails", body: "Rails is a web application framework"),
34+
search_result.new(title: "Love of Ruby", url: "/articles/love-of-ruby", body: "Ruby is a programming language")
35+
]
36+
37+
render_component(results: search_results)
38+
39+
expect(rendered).to have_content("Joy of Rails")
40+
expect(rendered).to have_css(%([href="/articles/joy-of-rails"]))
41+
expect(rendered).to have_content("Love of Ruby")
42+
expect(rendered).to have_css(%([href="/articles/love-of-ruby"]))
43+
end
44+
45+
it "raises an error when no component is registered for the result" do
46+
search_results = [
47+
search_result.new(title: "Joy of Rails", url: "/articles/joy-of-rails", body: "Rails is a web application framework"),
48+
search_result.new(title: "Love of Ruby", url: "/articles/love-of-ruby", body: "Ruby is a programming language")
49+
]
50+
51+
expect {
52+
render_component(results: search_results)
53+
}.to raise_error(ArgumentError, %r{\[Searches::Result\] No component registered})
54+
end
55+
end

0 commit comments

Comments
 (0)