Skip to content

Commit 4434ec0

Browse files
authoredSep 9, 2024··
[SelectPanel] Disallow passing role: argument (#3054)
1 parent 96b42db commit 4434ec0

File tree

4 files changed

+90
-4
lines changed

4 files changed

+90
-4
lines changed
 

‎.changeset/chilly-berries-retire.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/view-components': patch
3+
---
4+
5+
[SelectPanel] Disallow passing `role:` argument

‎app/components/primer/alpha/select_panel.rb

+31-4
Original file line numberDiff line numberDiff line change
@@ -250,10 +250,28 @@ module Alpha
250250
# )
251251
# ```
252252
class SelectPanel < Primer::Component
253+
# @private
254+
module Utils
255+
def raise_if_role_given!(**system_arguments)
256+
return if shouldnt_raise_error?
257+
return unless system_arguments.include?(:role)
258+
259+
raise(
260+
"Please avoid passing the `role:` argument to `SelectPanel` and its subcomponents. "\
261+
"The component will automatically apply the correct roles where necessary."
262+
)
263+
end
264+
end
265+
266+
include Utils
267+
253268
# The component that should be used to render the list of items in the body of a SelectPanel.
254269
class ItemList < Primer::Alpha::ActionList
270+
include Utils
271+
255272
# @param system_arguments [Hash] The arguments accepted by <%= link_to_component(Primer::Alpha::ActionList) %>.
256273
def initialize(**system_arguments)
274+
raise_if_role_given!(**system_arguments)
257275
select_variant = system_arguments[:select_variant] || Primer::Alpha::ActionList::DEFAULT_SELECT_VARIANT
258276

259277
super(
@@ -263,6 +281,16 @@ def initialize(**system_arguments)
263281
**system_arguments
264282
)
265283
end
284+
285+
def with_item(**system_arguments)
286+
raise_if_role_given!(**system_arguments)
287+
super
288+
end
289+
290+
def with_avatar_item(**system_arguments)
291+
raise_if_role_given!(**system_arguments)
292+
super
293+
end
266294
end
267295

268296
status :alpha
@@ -362,6 +390,8 @@ def initialize(
362390
anchor_side: Primer::Alpha::Overlay::DEFAULT_ANCHOR_SIDE,
363391
**system_arguments
364392
)
393+
raise_if_role_given!(**system_arguments)
394+
365395
if src.present?
366396
url = URI(src)
367397
query = url.query || ""
@@ -419,12 +449,9 @@ def initialize(
419449
form_arguments: form_arguments,
420450
id: "#{@panel_id}-list",
421451
select_variant: @select_variant,
422-
role: "listbox",
423-
aria_selection_variant: @select_variant == :multiple ? :checked : :selected,
424452
aria: {
425453
label: "#{title} options"
426-
},
427-
p: 2
454+
}
428455
)
429456
end
430457

‎app/components/primer/component.rb

+4
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ def should_raise_error?
141141
!Rails.env.production? && raise_on_invalid_options? && !ENV["PRIMER_WARNINGS_DISABLED"]
142142
end
143143

144+
def shouldnt_raise_error?
145+
!should_raise_error?
146+
end
147+
144148
def should_raise_aria_error?
145149
!Rails.env.production? && raise_on_invalid_aria? && !ENV["PRIMER_WARNINGS_DISABLED"]
146150
end

‎test/components/alpha/select_panel_test.rb

+50
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,56 @@ def test_renders_close_button
101101
panel_id = page.find_css("select-panel").first.attributes["id"].value
102102
assert_selector "select-panel button[data-close-dialog-id='#{panel_id}-dialog']"
103103
end
104+
105+
def test_raises_if_role_given
106+
with_raise_on_invalid_options(true) do
107+
error = assert_raises do
108+
render_inline(Primer::Alpha::SelectPanel.new(role: :listbox))
109+
end
110+
111+
assert_includes error.message, "Please avoid passing the `role:` argument"
112+
end
113+
end
114+
115+
def test_raises_if_role_given_to_item_slot
116+
with_raise_on_invalid_options(true) do
117+
error = assert_raises do
118+
render_inline(Primer::Alpha::SelectPanel.new) do |panel|
119+
panel.with_item(role: :option)
120+
end
121+
end
122+
123+
assert_includes error.message, "Please avoid passing the `role:` argument"
124+
end
125+
end
126+
127+
def test_does_not_raise_if_no_role_given_to_item_slot
128+
render_inline(Primer::Alpha::SelectPanel.new) do |panel|
129+
panel.with_item(label: "Foo")
130+
end
131+
132+
assert_selector "select-panel"
133+
end
134+
135+
def test_raises_if_role_given_to_avatar_item_slot
136+
with_raise_on_invalid_options(true) do
137+
error = assert_raises do
138+
render_inline(Primer::Alpha::SelectPanel.new) do |panel|
139+
panel.with_avatar_item(src: "camertron.jpg", username: "camertron", role: :option)
140+
end
141+
end
142+
143+
assert_includes error.message, "Please avoid passing the `role:` argument"
144+
end
145+
end
146+
147+
def test_does_not_raise_if_role_not_given_to_avatar_item_slot
148+
render_inline(Primer::Alpha::SelectPanel.new) do |panel|
149+
panel.with_avatar_item(src: "camertron.jpg", username: "camertron")
150+
end
151+
152+
assert_selector(".avatar-small")
153+
end
104154
end
105155
end
106156
end

0 commit comments

Comments
 (0)
Please sign in to comment.