Skip to content

Commit

Permalink
[SelectPanel] Fix tab index issue in multi-select mode (primer#3077)
Browse files Browse the repository at this point in the history
  • Loading branch information
camertron authored Sep 12, 2024
1 parent f4e65d4 commit 66488a3
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/curly-phones-burn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

[SelectPanel] Fix tab index issue in multi-select mode
8 changes: 3 additions & 5 deletions app/components/primer/alpha/select_panel_element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,7 @@ export class SelectPanelElement extends HTMLElement {
const itemContent = this.#getItemContent(item)
if (!itemContent) continue

if (!this.isItemHidden(item) && !setZeroTabIndex) {
setZeroTabIndex = true
} else {
itemContent.setAttribute('tabindex', '-1')
}
itemContent.setAttribute('tabindex', '-1')

// <li> elements should not themselves be tabbable
item.removeAttribute('tabindex')
Expand Down Expand Up @@ -742,6 +738,8 @@ export class SelectPanelElement extends HTMLElement {
return true
}

if (!this.bannerErrorElement) return false

return !this.bannerErrorElement.hasAttribute('hidden')
}

Expand Down
30 changes: 30 additions & 0 deletions test/system/alpha/select_panel_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ def wait_for_dialog_ready
end

def filter_results(query:)
input = find("input")

# If the query is an empty string or nil, using fill_in does not
# trigger the right type of input event, which in turn prevents
# the remote-input element from firing its remote-input-success
# event. Instead we have to focus on the input field, select all
# the text, and press backspace. Doing so fires the right type of
# event and clears the input.
if query.blank?
old_active_element = active_element
input.evaluate_script("this.focus()")
keyboard.type([:control, "a"], :backspace)
old_active_element.evaluate_script("this.focus()")
end

find("input").fill_in(with: query)
end

Expand Down Expand Up @@ -1052,6 +1067,21 @@ def test_arrowing_skips_filtered_items
end
end

def test_can_tab_to_first_item_after_filtering
visit_preview(:local_fetch)

click_on_invoker_button

filter_results(query: "2")
assert_selector "select-panel ul li", count: 1

filter_results(query: "")
keyboard.type(:tab)
assert_equal active_element.text, "Item 1"
end

########## FORM TESTS ############

def test_single_select_form
visit_preview(:single_select_form, route_format: :json)

Expand Down

0 comments on commit 66488a3

Please sign in to comment.