Skip to content

Commit fac1ec9

Browse files
authoredAug 6, 2024··
Ensure pressing 'Enter' in SelectPanel filter input causes navigation if first item is link (#2978)
Co-authored-by: camertron <camertron@users.noreply.github.com>
1 parent 22936ff commit fac1ec9

File tree

16 files changed

+102
-8
lines changed

16 files changed

+102
-8
lines changed
 

‎.changeset/perfect-dogs-tell.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/view-components': patch
3+
---
4+
5+
Ensure pressing 'Enter' in SelectPanel filter input causes navigation if first item is link.

‎app/components/primer/alpha/select_panel_element.ts

+13-8
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ type SelectedItem = {
1111
label: string | null | undefined
1212
value: string | null | undefined
1313
inputName: string | null | undefined
14-
element: SelectPanelItem
1514
}
1615

1716
const validSelectors = ['[role="option"]']
@@ -382,6 +381,7 @@ export class SelectPanelElement extends HTMLElement {
382381
}
383382
}
384383
}
384+
385385
this.#updateInput()
386386
}
387387

@@ -396,7 +396,6 @@ export class SelectPanelElement extends HTMLElement {
396396
value,
397397
label: itemContent.querySelector('.ActionListItem-label')?.textContent?.trim(),
398398
inputName: itemContent.getAttribute('data-input-name'),
399-
element: item,
400399
})
401400
}
402401
}
@@ -632,7 +631,8 @@ export class SelectPanelElement extends HTMLElement {
632631
const item = this.visibleItems[0] as HTMLLIElement | null
633632

634633
if (item) {
635-
this.#handleItemActivated(item, false)
634+
const itemContent = this.#getItemContent(item)
635+
if (itemContent) itemContent.click()
636636
}
637637
} else if (key === 'ArrowDown') {
638638
const item = (this.focusableItem || this.#getItemContent(this.visibleItems[0])) as HTMLLIElement
@@ -645,13 +645,15 @@ export class SelectPanelElement extends HTMLElement {
645645
const item = this.visibleItems[0] as HTMLLIElement | null
646646

647647
if (item) {
648-
this.#getItemContent(item)!.focus()
648+
const itemContent = this.#getItemContent(item)
649+
if (itemContent) itemContent.focus()
649650
event.preventDefault()
650651
}
651652
} else if (key === 'End') {
652653
if (this.visibleItems.length > 0) {
653654
const item = this.visibleItems[this.visibleItems.length - 1] as HTMLLIElement
654-
this.#getItemContent(item)!.focus()
655+
const itemContent = this.#getItemContent(item)
656+
if (itemContent) itemContent.focus()
655657
event.preventDefault()
656658
}
657659
}
@@ -838,7 +840,7 @@ export class SelectPanelElement extends HTMLElement {
838840
dialog.addEventListener('cancel', handleDialogClose, {signal})
839841
}
840842

841-
#handleItemActivated(item: SelectPanelItem, shouldClose: boolean = true) {
843+
#handleItemActivated(item: SelectPanelItem) {
842844
// Hide popover after current event loop to prevent changes in focus from
843845
// altering the target of the event. Not doing this specifically affects
844846
// <a> tags. It causes the event to be sent to the currently focused element
@@ -847,7 +849,7 @@ export class SelectPanelElement extends HTMLElement {
847849
// works fine.
848850
if (this.selectVariant !== 'multiple') {
849851
setTimeout(() => {
850-
if (this.open && shouldClose) {
852+
if (this.open) {
851853
this.hide()
852854
}
853855
})
@@ -872,10 +874,13 @@ export class SelectPanelElement extends HTMLElement {
872874
const itemContent = this.#getItemContent(item)
873875

874876
if (this.selectVariant === 'single') {
875-
const element = this.selectedItems[0]?.element
877+
const value = this.selectedItems[0]?.value
878+
const element = this.visibleItems.find(el => this.#getItemContent(el)?.getAttribute('data-value') === value)
879+
876880
if (element) {
877881
this.#getItemContent(element)?.setAttribute(this.ariaSelectionType, 'false')
878882
}
883+
879884
this.#selectedItems.clear()
880885

881886
// Only check, never uncheck here. Single-select mode does not allow unchecking a checked item.

‎previews/primer/alpha/select_panel_preview.rb

+8
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,14 @@ def single_select_form(open_on_load: false, route_format: :html)
237237
def multiselect_form(open_on_load: false, route_format: :html)
238238
render_with_template(locals: { open_on_load: open_on_load, route_format: route_format })
239239
end
240+
241+
# @label List of links
242+
#
243+
# @snapshot interactive
244+
# @param open_on_load toggle
245+
def list_of_links(open_on_load: false)
246+
render_with_template(locals: { open_on_load: open_on_load })
247+
end
240248
end
241249
end
242250
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<% subject_id = SecureRandom.hex %>
2+
3+
<%= render(Primer::Alpha::SelectPanel.new(
4+
data: { interaction_subject: subject_id },
5+
select_variant: :single,
6+
fetch_strategy: :local,
7+
open_on_load: open_on_load
8+
)) do |panel| %>
9+
<% panel.with_show_button { "Panel" } %>
10+
<% panel.with_item(label: "GitHub", href: "https://github.com") %>
11+
<% panel.with_item(label: "Microsoft", href: "https://microsoft.com") %>
12+
<% panel.with_item(label: "Primer", href: "https://primer.style") %>
13+
<% panel.with_item(label: "Catalyst", href: "https://catalyst.rocks") %>
14+
<% end %>
15+
16+
<%= render partial: "primer/alpha/select_panel_preview/interaction_subject_js", locals: { subject_id: subject_id } %>

‎static/info_arch.json

+13
Original file line numberDiff line numberDiff line change
@@ -8105,6 +8105,19 @@
81058105
"color-contrast"
81068106
]
81078107
}
8108+
},
8109+
{
8110+
"preview_path": "primer/alpha/select_panel/list_of_links",
8111+
"name": "list_of_links",
8112+
"snapshot": "interactive",
8113+
"skip_rules": {
8114+
"wont_fix": [
8115+
"region"
8116+
],
8117+
"will_fix": [
8118+
"color-contrast"
8119+
]
8120+
}
81088121
}
81098122
],
81108123
"subcomponents": [

‎static/previews.json

+13
Original file line numberDiff line numberDiff line change
@@ -6160,6 +6160,19 @@
61606160
"color-contrast"
61616161
]
61626162
}
6163+
},
6164+
{
6165+
"preview_path": "primer/alpha/select_panel/list_of_links",
6166+
"name": "list_of_links",
6167+
"snapshot": "interactive",
6168+
"skip_rules": {
6169+
"wont_fix": [
6170+
"region"
6171+
],
6172+
"will_fix": [
6173+
"color-contrast"
6174+
]
6175+
}
61636176
}
61646177
]
61656178
},

‎test/system/alpha/select_panel_test.rb

+34
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,18 @@ def test_pressing_enter_in_filter_input_checks_first_item
254254
refute_selector "[aria-selected]"
255255
end
256256

257+
def test_pressing_enter_in_filter_input_navigates_if_first_item_is_link
258+
visit_preview(:list_of_links)
259+
260+
click_on_invoker_button
261+
262+
assert_equal active_element.tag_name, "input"
263+
264+
keyboard.type(:enter)
265+
266+
assert_current_path "https://github.com"
267+
end
268+
257269
########## SINGLE SELECT TESTS ############
258270

259271
def test_single_select_item_checked
@@ -360,6 +372,28 @@ def test_single_select_item_unchecks_previously_checked_item
360372
refute_selector "[aria-checked]", visible: :hidden
361373
end
362374

375+
def test_single_select_item_unchecks_previously_checked_item_after_filtering
376+
visit_preview(:playground)
377+
378+
click_on_invoker_button
379+
380+
# clicking item closes panel, so checked item is hidden
381+
assert_selector "[aria-selected=true]", text: "Phaser"
382+
refute_selector "[aria-checked]"
383+
384+
wait_for_items_to_load do
385+
filter_results(query: "ph")
386+
end
387+
388+
click_on "Photon torpedo"
389+
390+
click_on_invoker_button
391+
# clicking item closes panel, so checked item is hidden
392+
assert_selector "[aria-selected=false]", text: "Phaser"
393+
assert_selector "[aria-selected=true]", text: "Photon torpedo"
394+
refute_selector "[aria-checked]"
395+
end
396+
363397
def test_single_selected_item_cannot_be_unchecked
364398
visit_preview(:single_select)
365399

0 commit comments

Comments
 (0)
Please sign in to comment.