Skip to content

Commit

Permalink
Ensure pressing 'Enter' in SelectPanel filter input causes navigation…
Browse files Browse the repository at this point in the history
… if first item is link (#2978)

Co-authored-by: camertron <camertron@users.noreply.github.com>
  • Loading branch information
camertron and camertron committed Aug 6, 2024
1 parent 22936ff commit fac1ec9
Show file tree
Hide file tree
Showing 16 changed files with 102 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/perfect-dogs-tell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Ensure pressing 'Enter' in SelectPanel filter input causes navigation if first item is link.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 13 additions & 8 deletions app/components/primer/alpha/select_panel_element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ type SelectedItem = {
label: string | null | undefined
value: string | null | undefined
inputName: string | null | undefined
element: SelectPanelItem
}

const validSelectors = ['[role="option"]']
Expand Down Expand Up @@ -382,6 +381,7 @@ export class SelectPanelElement extends HTMLElement {
}
}
}

this.#updateInput()
}

Expand All @@ -396,7 +396,6 @@ export class SelectPanelElement extends HTMLElement {
value,
label: itemContent.querySelector('.ActionListItem-label')?.textContent?.trim(),
inputName: itemContent.getAttribute('data-input-name'),
element: item,
})
}
}
Expand Down Expand Up @@ -632,7 +631,8 @@ export class SelectPanelElement extends HTMLElement {
const item = this.visibleItems[0] as HTMLLIElement | null

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

if (item) {
this.#getItemContent(item)!.focus()
const itemContent = this.#getItemContent(item)
if (itemContent) itemContent.focus()
event.preventDefault()
}
} else if (key === 'End') {
if (this.visibleItems.length > 0) {
const item = this.visibleItems[this.visibleItems.length - 1] as HTMLLIElement
this.#getItemContent(item)!.focus()
const itemContent = this.#getItemContent(item)
if (itemContent) itemContent.focus()
event.preventDefault()
}
}
Expand Down Expand Up @@ -838,7 +840,7 @@ export class SelectPanelElement extends HTMLElement {
dialog.addEventListener('cancel', handleDialogClose, {signal})
}

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

if (this.selectVariant === 'single') {
const element = this.selectedItems[0]?.element
const value = this.selectedItems[0]?.value
const element = this.visibleItems.find(el => this.#getItemContent(el)?.getAttribute('data-value') === value)

if (element) {
this.#getItemContent(element)?.setAttribute(this.ariaSelectionType, 'false')
}

this.#selectedItems.clear()

// Only check, never uncheck here. Single-select mode does not allow unchecking a checked item.
Expand Down
8 changes: 8 additions & 0 deletions previews/primer/alpha/select_panel_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,14 @@ def single_select_form(open_on_load: false, route_format: :html)
def multiselect_form(open_on_load: false, route_format: :html)
render_with_template(locals: { open_on_load: open_on_load, route_format: route_format })
end

# @label List of links
#
# @snapshot interactive
# @param open_on_load toggle
def list_of_links(open_on_load: false)
render_with_template(locals: { open_on_load: open_on_load })
end
end
end
end
16 changes: 16 additions & 0 deletions previews/primer/alpha/select_panel_preview/list_of_links.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<% subject_id = SecureRandom.hex %>
<%= render(Primer::Alpha::SelectPanel.new(
data: { interaction_subject: subject_id },
select_variant: :single,
fetch_strategy: :local,
open_on_load: open_on_load
)) do |panel| %>
<% panel.with_show_button { "Panel" } %>
<% panel.with_item(label: "GitHub", href: "https://github.com") %>
<% panel.with_item(label: "Microsoft", href: "https://microsoft.com") %>
<% panel.with_item(label: "Primer", href: "https://primer.style") %>
<% panel.with_item(label: "Catalyst", href: "https://catalyst.rocks") %>
<% end %>
<%= render partial: "primer/alpha/select_panel_preview/interaction_subject_js", locals: { subject_id: subject_id } %>
13 changes: 13 additions & 0 deletions static/info_arch.json
Original file line number Diff line number Diff line change
Expand Up @@ -8105,6 +8105,19 @@
"color-contrast"
]
}
},
{
"preview_path": "primer/alpha/select_panel/list_of_links",
"name": "list_of_links",
"snapshot": "interactive",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
}
],
"subcomponents": [
Expand Down
13 changes: 13 additions & 0 deletions static/previews.json
Original file line number Diff line number Diff line change
Expand Up @@ -6160,6 +6160,19 @@
"color-contrast"
]
}
},
{
"preview_path": "primer/alpha/select_panel/list_of_links",
"name": "list_of_links",
"snapshot": "interactive",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
}
]
},
Expand Down
34 changes: 34 additions & 0 deletions test/system/alpha/select_panel_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,18 @@ def test_pressing_enter_in_filter_input_checks_first_item
refute_selector "[aria-selected]"
end

def test_pressing_enter_in_filter_input_navigates_if_first_item_is_link
visit_preview(:list_of_links)

click_on_invoker_button

assert_equal active_element.tag_name, "input"

keyboard.type(:enter)

assert_current_path "https://github.com"
end

########## SINGLE SELECT TESTS ############

def test_single_select_item_checked
Expand Down Expand Up @@ -360,6 +372,28 @@ def test_single_select_item_unchecks_previously_checked_item
refute_selector "[aria-checked]", visible: :hidden
end

def test_single_select_item_unchecks_previously_checked_item_after_filtering
visit_preview(:playground)

click_on_invoker_button

# clicking item closes panel, so checked item is hidden
assert_selector "[aria-selected=true]", text: "Phaser"
refute_selector "[aria-checked]"

wait_for_items_to_load do
filter_results(query: "ph")
end

click_on "Photon torpedo"

click_on_invoker_button
# clicking item closes panel, so checked item is hidden
assert_selector "[aria-selected=false]", text: "Phaser"
assert_selector "[aria-selected=true]", text: "Photon torpedo"
refute_selector "[aria-checked]"
end

def test_single_selected_item_cannot_be_unchecked
visit_preview(:single_select)

Expand Down

0 comments on commit fac1ec9

Please sign in to comment.