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 ActionList dividers to be added manually #1875

Merged
merged 5 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from all 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/selfish-glasses-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Allow ActionList dividers to be added manually
3 changes: 2 additions & 1 deletion app/components/primer/alpha/action_list.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
<% end %>
<%= render(Primer::BaseComponent.new(tag: :ul, **@system_arguments)) do %>
<% items.each_with_index do |item, index| %>
<% if index > 0 && @show_dividers %>
<%# the conditions here make sure two dividers are never rendered one after the other %>
<% if index > 0 && @show_dividers && !item.is_a?(Divider) && !items[index - 1].is_a?(Divider) %>
<%= render(Primer::Alpha::ActionList::Divider.new) %>
<% end %>
<%= item %>
Expand Down
14 changes: 12 additions & 2 deletions app/components/primer/alpha/action_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ def custom_element_name
end
}

# Adds a divider to the list of items.
#
# @param system_arguments [Hash] The arguments accepted by <%= link_to_component(Primer::Alpha::ActionList::Divider) %>.
def with_divider(**system_arguments, &block)
# This is a giant hack that should be removed when :items can be converted into a polymorphic slot.
# This feature needs to land in view_component first: https://github.com/ViewComponent/view_component/pull/1652
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 A pretty wild hack

set_slot(:items, { renderable: Divider, collection: true }, **system_arguments, &block)
end

# @param role [Boolean] ARIA role describing the function of the list. listbox and menu are a common values.
# @param item_classes [String] Additional CSS classes to attach to items.
# @param scheme [Symbol] <%= one_of(Primer::Alpha::ActionList::SCHEME_OPTIONS) %>. `inset` children are offset (vertically and horizontally) from list edges. `full` (default) children are flush (vertically and horizontally) with list edges.
Expand Down Expand Up @@ -81,12 +90,13 @@ def initialize(
# @private
def before_render
aria_label = aria(:label, @system_arguments)
aria_labelledby = aria(:labelledby, @system_arguments)

if heading.present?
@system_arguments[:"aria-labelledby"] = @id
raise ArgumentError, "An aria-label should not be provided if a heading is present" if aria_label.present?
elsif aria_label.blank?
raise ArgumentError, "An aria-label or heading must be provided"
elsif aria_label.blank? && aria_labelledby.blank?
raise ArgumentError, "An aria-label, aria-labelledby, or heading must be provided"
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/components/primer/alpha/action_list/divider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class Divider < Primer::Component
# @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
def initialize(scheme: DEFAULT_SCHEME, **system_arguments)
@system_arguments = system_arguments
@system_arguments[:tag] = :div
@system_arguments[:role] = :separator
@system_arguments[:tag] = :li
@system_arguments[:role] = :presentation
@system_arguments[:'aria-hidden'] = true
@scheme = fetch_or_fallback(SCHEME_OPTIONS, scheme, DEFAULT_SCHEME)
@system_arguments[:classes] = class_names(
Expand Down
33 changes: 33 additions & 0 deletions previews/primer/alpha/action_list_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,39 @@ def leading_visuals(
end
end

# @label With manual dividers
#
# @param role text
# @param scheme [Symbol] select [full, inset]
# @param show_dividers toggle
def with_manual_dividers(
role: Primer::Alpha::ActionList::DEFAULT_ROLE,
scheme: Primer::Alpha::ActionList::DEFAULT_SCHEME,
show_dividers: false
)
render(Primer::Alpha::ActionList.new(
role: role,
scheme: scheme,
show_dividers: show_dividers
)) do |component|
component.with_heading(title: "Action List")
component.with_item(label: "Item one", href: "/") do |item|
item.with_leading_visual_icon(icon: :gear)
end
component.with_divider
component.with_item(label: "Item two", href: "/") do |item|
item.with_leading_visual_icon(icon: :star)
end
component.with_item(label: "Item three", href: "/") do |item|
item.with_leading_visual_icon(icon: :heart)
end
component.with_divider
component.with_item(label: "Item four", href: "/") do |item|
item.with_leading_visual_icon(icon: :bug)
end
end
end

# @label Divider
#
# @param scheme [Symbol] select [subtle, filled]
Expand Down
28 changes: 27 additions & 1 deletion test/components/alpha/action_list_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def test_invalid_list
render_inline(Primer::Alpha::ActionList.new)
end

assert_includes(error.message, "label or heading must be provided")
assert_includes(error.message, "aria-label, aria-labelledby, or heading must be provided")
end

def test_active_item
Expand Down Expand Up @@ -83,6 +83,32 @@ def test_heading_denies_tag_argument

assert_match(/This component has a fixed tag/, error.message)
end

def test_manual_dividers
render_inline(Primer::Alpha::ActionList.new(aria: { label: "List" })) do |component|
component.with_item(label: "Item 1", href: "/item1")
component.with_item(label: "Item 2", href: "/item2")
component.with_divider
component.with_item(label: "Item 3", href: "/item3")
end

list_items = page.find_css("ul.ActionListWrap li").map do |list_item|
classes = list_item["class"].split

if classes.include?("ActionListItem")
{ type: :item, href: list_item.css("a").first["href"] }
elsif classes.include?("ActionList-sectionDivider")
{ type: :divider }
end
end

assert_equal list_items, [
{ type: :item, href: "/item1" },
{ type: :item, href: "/item2" },
{ type: :divider },
{ type: :item, href: "/item3" }
]
end
end
end
end