Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow NavList selection/deselection by id/href in javascript #1822

Merged
merged 9 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tender-islands-peel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Allow NavList selection/deselection by id/href in javascript
7 changes: 7 additions & 0 deletions app/components/primer/alpha/action_list/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ def initialize(
@system_arguments[:aria] ||= {}
@system_arguments[:aria][:disabled] = "true" if @disabled

@system_arguments[:data] ||= {}
@system_arguments[:data][:targets] = "#{list_class.custom_element_name}.items"

@label_arguments = {
classes: class_names(
label_classes,
Expand Down Expand Up @@ -236,6 +239,10 @@ def before_render
"ActionListContent--blockDescription" => description && @description_scheme == :block
)
end

def list_class
Primer::Alpha::ActionList
end
end
end
end
Expand Down
5 changes: 5 additions & 0 deletions app/components/primer/alpha/nav_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ module Alpha
class NavList < Primer::Component
status :alpha

# @private
def self.custom_element_name
"nav-list"
end

# Sections. Each section is a list of links and an optional heading.
#
# @param system_arguments [Hash] The arguments accepted by <%= link_to_component(Primer::Alpha::NavList::Section) %>.
Expand Down
108 changes: 105 additions & 3 deletions app/components/primer/alpha/nav_list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
import {controller, target, targets} from '@github/catalyst'

@controller
class NavListElement extends HTMLElement {
export class NavListElement extends HTMLElement {
@target list: HTMLElement
@targets items: HTMLElement[]
@target showMoreItem: HTMLElement
@targets focusMarkers: HTMLElement[]

Expand Down Expand Up @@ -40,6 +41,43 @@ class NavListElement extends HTMLElement {
return this.showMoreItem.getAttribute('src') || ''
}

selectItemById(itemId: string | null): boolean {
if (!itemId) return false

const selectedItem = this.#findSelectedNavItemById(itemId)

if (selectedItem) {
this.#select(selectedItem)
return true
}

return false
}

selectItemByHref(href: string | null): boolean {
if (!href) return false

const selectedItem = this.#findSelectedNavItemByHref(href)

if (selectedItem) {
this.#select(selectedItem)
return true
}

return false
}

selectItemByCurrentLocation(): boolean {
const selectedItem = this.#findSelectedNavItemByCurrentLocation()

if (selectedItem) {
this.#select(selectedItem)
return true
}

return false
}

// expand collapsible item onClick
expandItem(item: HTMLElement) {
item.nextElementSibling?.removeAttribute('data-hidden')
Expand Down Expand Up @@ -95,7 +133,7 @@ class NavListElement extends HTMLElement {
this.currentPage--
return
}
const fragment = this.parseHTML(document, html)
const fragment = this.#parseHTML(document, html)
fragment?.querySelector('li > a')?.setAttribute('data-targets', 'nav-list.focusMarkers')
this.list.insertBefore(fragment, this.showMoreItem)
this.focusMarkers.pop()?.focus()
Expand All @@ -114,12 +152,76 @@ class NavListElement extends HTMLElement {
}
}

private parseHTML(document: Document, html: string): DocumentFragment {
#parseHTML(document: Document, html: string): DocumentFragment {
const template = document.createElement('template')
// eslint-disable-next-line github/no-inner-html
template.innerHTML = html
return document.importNode(template.content, true)
}

#findSelectedNavItemById(itemId: string): HTMLElement | null {
// First we compare the selected link to data-item-id for each nav item
for (const navItem of this.items) {
const keys = navItem.getAttribute('data-item-id')?.split(' ') || []

if (keys.includes(itemId)) {
return navItem
}
}

return null
}

#findSelectedNavItemByHref(href: string): HTMLElement | null {
// If we didn't find a match, we compare the selected link to the href of each nav item
const selectedNavItem = this.querySelector<HTMLAnchorElement>(`.ActionListContent[href="${href}"]`)
if (selectedNavItem) {
return selectedNavItem.closest('.ActionListItem')
}

return null
}

#findSelectedNavItemByCurrentLocation(): HTMLElement | null {
return this.#findSelectedNavItemByHref(window.location.pathname)
}

#select(navItem: HTMLElement) {
const currentlySelectedItem = this.querySelector('.ActionListItem--navActive') as HTMLElement
if (currentlySelectedItem) this.#deselect(currentlySelectedItem)

navItem.classList.add('ActionListItem--navActive')

const parentMenu = this.#findParentMenu(navItem)

if (parentMenu) {
this.expandItem(parentMenu)
parentMenu.classList.add('ActionListContent--hasActiveSubItem')
}
}

#deselect(navItem: HTMLElement) {
navItem.classList.remove('ActionListItem--navActive')

const parentMenu = this.#findParentMenu(navItem)

if (parentMenu) {
this.collapseItem(parentMenu)
parentMenu.classList.remove('ActionListContent--hasActiveSubItem')
}
}

#findParentMenu(navItem: HTMLElement): HTMLElement | null {
if (!navItem.classList.contains('ActionListItem--subItem')) return null

const parent = navItem.closest('li.ActionListItem--hasSubItem')?.querySelector('button.ActionListContent')

if (parent) {
return parent as HTMLElement
} else {
return null
}
}
}

declare global {
Expand Down
4 changes: 4 additions & 0 deletions app/components/primer/alpha/nav_list/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ def active_sub_item?
def current_page?(url)
helpers.current_page?(url)
end

def list_class
Primer::Alpha::NavList
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/components/primer/alpha/nav_list/section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Section < ActionList

# @private
def self.custom_element_name
"nav-list"
Primer::Alpha::NavList.custom_element_name
end

# @param selected_item_id [Symbol] The ID of the currently selected item. Used internally.
Expand Down
98 changes: 98 additions & 0 deletions test/system/alpha/nav_list_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,103 @@ def test_shows_more_items
assert_selector "li", text: "Bachelor Chow"
assert_selector "li", text: "LöBrau"
end

def test_js_api_allows_selecting_item_by_id
visit_preview(:default)

select_item_by_id(:collaborators)
assert_item_id_selected(:collaborators)
end

def test_selecting_id_collapses_section
visit_preview(:default)

select_item_by_id(:collaborators)
assert_selector "button[aria-expanded=false]", text: "Moderation"
end

def test_js_api_allows_selecting_item_by_href
visit_preview(:default)

select_item_by_href("/collaborators")
assert_item_href_selected("/collaborators")
end

def test_selecting_href_collapses_section
visit_preview(:default)

select_item_by_href("/collaborators")
assert_selector "button[aria-expanded=false]", text: "Moderation"
end

def test_js_api_allows_selecting_item_by_current_location
visit_preview(:default)

# set the URL without reloading the page
page.evaluate_script(<<~JS)
(() => {
window.history.pushState({}, "", "http://localhost/collaborators")
})();
JS

select_item_by_current_location
assert_item_href_selected("/collaborators")
assert_selector "button[aria-expanded=false]", text: "Moderation"
end

private

def select_item_by_id(id)
select_item_by(id: id)
end

def select_item_by_href(href)
select_item_by(href: href)
end

def select_item_by_current_location
page.evaluate_script(<<~JS)
(() => {
const navLists = document.querySelectorAll('nav-list');

// Unfortunately the NavList component emits multiple <nav-list> elements,
// so we have to loop over all of them. This will be addressed in the very
// near future.
for (const navList of navLists) {
if (navList.selectItemByCurrentLocation()) {
return;
}
}
})();
JS
end

def select_item_by(id: nil, href: nil)
func = id ? "selectItemById" : "selectItemByHref"
param = id || href

page.evaluate_script(<<~JS)
(() => {
const navLists = document.querySelectorAll('nav-list');

// Unfortunately the NavList component emits multiple <nav-list> elements,
// so we have to loop over all of them. This will be addressed in the very
// near future.
for (const navList of navLists) {
if (navList.#{func}('#{param}')) {
return;
}
}
})();
JS
end

def assert_item_id_selected(item_id)
assert_selector("li[data-item-id='#{item_id}'].ActionListItem--navActive")
end

def assert_item_href_selected(item_href)
assert_selector("li.ActionListItem--navActive a[href='#{item_href}']")
end
end
end
2 changes: 1 addition & 1 deletion test/system/test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

module System
class TestCase < ActionDispatch::SystemTestCase
driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: { process_timeout: 240, timeout: 240 }
driven_by :primer_cuprite, using: :chrome, screen_size: [1400, 1400], options: { process_timeout: 240, timeout: 240 }

# Skip `:region` which relates to preview page structure rather than individual component.
# Skip `:color-contrast` which requires primer design-level change.
Expand Down
2 changes: 1 addition & 1 deletion test/test_helpers/cuprite_setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

# Then, we need to register our driver to be able to use it later
# with #driven_by method.
Capybara.register_driver(:cuprite) do |app|
Capybara.register_driver(:primer_cuprite) do |app|
Capybara::Cuprite::Driver.new(
app,
**{
Expand Down