Skip to content

Commit

Permalink
Fix action bar issue where keys don't loop around to end (primer#2292)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonrohan authored Oct 18, 2023
1 parent d7eafca commit 46e3ff0
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/poor-rats-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Fix ActionBar issue where left and end key don't loop around to end of the action bar items.
9 changes: 8 additions & 1 deletion app/components/primer/alpha/action_bar_element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,18 @@ class ActionBarElement extends HTMLElement {
bindKeys: FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd,
focusOutBehavior: 'wrap',
focusableElementFilter: element => {
return !element.closest('.ActionBar-item[hidden]') && !element.closest('li.ActionListItem')
return this.#isVisible(element)
}
})
}

#isVisible(element: HTMLElement): boolean {
// Safari doesn't support `checkVisibility` yet.
if (typeof element.checkVisibility === 'function') return element.checkVisibility()

return Boolean(element.offsetParent || element.offsetWidth || element.offsetHeight)
}

#itemGap(): number {
return parseInt(window.getComputedStyle(this.itemContainer)?.columnGap, 10) || 0
}
Expand Down
10 changes: 10 additions & 0 deletions test/system/alpha/action_bar_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,14 @@ def test_click_outside_of_menu_sets_tabindex_back
assert_equal page.evaluate_script("document.activeElement.classList.contains('Button--iconOnly')"), true
assert_equal page.evaluate_script("!!document.activeElement.closest('action-menu')"), true
end

def test_arrow_left_loops_to_last_item
visit_preview(:default)

# Tab to first item and press left arrow
page.driver.browser.keyboard.type(:tab, :left)

# The last item "Attach" should be focused
assert page.evaluate_script("document.activeElement.querySelector('svg.octicon-paperclip')")
end
end

0 comments on commit 46e3ff0

Please sign in to comment.