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

[SelectPanel] Raise an error when remote fetch + hidden filter combo #3053

Merged
merged 7 commits into from
Sep 9, 2024
Merged
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
5 changes: 5 additions & 0 deletions .changeset/many-zoos-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

[SelectPanel] Raise an error when remote fetch + hidden filter argument combo
12 changes: 12 additions & 0 deletions app/components/primer/alpha/select_panel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,18 @@ def initialize(
label: "#{title} options"
}
)

return if @show_filter || @fetch_strategy != :remote
return if shouldnt_raise_error?

raise(
"Hiding the filter input with a remote fetch strategy is not permitted, "\
"since such a combinaton of options will cause the component to only "\
"fetch items from the server once when the panel opens for the first time; "\
"this is what the `:eventually_local` fetch strategy is designed to do. "\
"Consider passing `show_filter: true` or use the `:eventually_local` fetch "\
"strategy instead."
)
end

# @!parse
Expand Down
14 changes: 7 additions & 7 deletions previews/primer/alpha/select_panel_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class SelectPanelPreview < ViewComponent::Preview
# @param dynamic_label toggle
# @param dynamic_label_prefix text
# @param dynamic_aria_label_prefix text
# @param show_filter toggle
# @param open_on_load toggle
# @param anchor_align [Symbol] select [start, center, end]
# @param anchor_side [Symbol] select [outside_bottom, outside_top, outside_left, outside_right]
Expand All @@ -29,7 +28,6 @@ def playground(
dynamic_label: false,
dynamic_label_prefix: nil,
dynamic_aria_label_prefix: nil,
show_filter: true,
open_on_load: false,
anchor_align: :start,
anchor_side: :outside_bottom,
Expand All @@ -47,7 +45,6 @@ def playground(
dynamic_label: dynamic_label,
dynamic_label_prefix: dynamic_label_prefix,
dynamic_aria_label_prefix: dynamic_aria_label_prefix,
show_filter: show_filter,
open_on_load: open_on_load,
anchor_align: anchor_align,
anchor_side: anchor_side
Expand All @@ -59,18 +56,21 @@ def playground(
#
# @snapshot interactive
# @param open_on_load toggle
def default(open_on_load: false)
# @param show_filter toggle
def default(open_on_load: false, show_filter: true)
render_with_template(template: "primer/alpha/select_panel_preview/local_fetch", locals: {
open_on_load: open_on_load
open_on_load: open_on_load,
show_filter: show_filter
})
end

# @label Local fetch
#
# @snapshot interactive
# @param open_on_load toggle
def local_fetch(open_on_load: false)
render_with_template(locals: { open_on_load: open_on_load })
# @param show_filter toggle
def local_fetch(open_on_load: false, show_filter: true)
render_with_template(locals: { open_on_load: open_on_load, show_filter: show_filter })
end

# @label Eventually local fetch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
data: { interaction_subject: subject_id },
select_variant: :multiple,
fetch_strategy: :local,
open_on_load: open_on_load
open_on_load: open_on_load,
show_filter: show_filter,
)) do |panel| %>
<% panel.with_show_button { "Panel" } %>
<% panel.with_item(label: "Item 1") %>
Expand Down
12 changes: 11 additions & 1 deletion test/components/alpha/select_panel_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_renders_a_filter_input
end

def test_does_not_render_a_filter_input
render_inline(Primer::Alpha::SelectPanel.new(show_filter: false))
render_inline(Primer::Alpha::SelectPanel.new(fetch_strategy: :local, show_filter: false))

refute_selector("primer-text-field")
end
Expand Down Expand Up @@ -102,6 +102,16 @@ def test_renders_close_button
assert_selector "select-panel button[data-close-dialog-id='#{panel_id}-dialog']"
end

def test_raises_if_remote_strategy_and_hidden_filter_used_together
with_raise_on_invalid_options(true) do
error = assert_raises do
render_inline(Primer::Alpha::SelectPanel.new(fetch_strategy: :remote, show_filter: false))
end

assert_includes error.message, "Hiding the filter input with a remote fetch strategy is not permitted"
end
end

def test_raises_if_role_given
with_raise_on_invalid_options(true) do
error = assert_raises do
Expand Down
Loading