Skip to content

Commit

Permalink
Merge branch 'main' into jshark/select-panel-banner-scheme
Browse files Browse the repository at this point in the history
  • Loading branch information
jamieshark authored Sep 12, 2024
2 parents 645aa25 + 66488a3 commit 68b73ee
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 16 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
17 changes: 7 additions & 10 deletions demo/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion demo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"version": "0.1.0",
"dependencies": {
"@primer/css": "^21.3.6",
"@primer/primitives": "^9.0.3",
"@primer/primitives": "^9.1.0",
"@rails/actioncable": "^7.2.100",
"@rails/ujs": "^7.1.400",
"turbolinks": "^5.2.0",
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 68b73ee

Please sign in to comment.