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.

Sign up for GitHub

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

Jump to bottom

Add parenthesis to trailing counter #2807

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Add parenthesis to trailing counter #2807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
554de3f
Add parenthesis to trailing counter
khiga8 Apr 26, 2024
4b5b1e0
Remove extra span nesting
khiga8 Apr 26, 2024
1a86e1f
Create brown-boats-hunt.md
khiga8 Apr 26, 2024
cf3044c
Merge branch 'main' into kh-add-paranthesis
khiga8 Apr 29, 2024
307f7c7
Merge branch 'main' into kh-add-paranthesis
khiga8 Apr 30, 2024
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/brown-boats-hunt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Add parenthesis to trailing counter for React parity, and to improve screen reader speech output
7 changes: 6 additions & 1 deletion app/components/primer/beta/button.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
<span class="Button-label"><%= trimmed_content %></span>
<% if trailing_visual %>
<span class="Button-visual Button-trailingVisual">
<%= trailing_visual %>
<% if @trailing_visual_counter %>
<span class="sr-only">(<%= trailing_visual %>)</span>
<%= trailing_visual %>
<% else %>
<%= trailing_visual %>
<% end %>
</span>
<% end %>
</span>
Expand Down
5 changes: 4 additions & 1 deletion app/components/primer/beta/button.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ class Button < Primer::Component
renders_one :trailing_visual, types: {
icon: Primer::Beta::Octicon,
label: Primer::Beta::Label,
counter: Primer::Beta::Counter
counter: lambda { |**system_arguments|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pls lmk if there is a better way to know that the trailing_visual is a counter, without setting this boolean.

@trailing_visual_counter = true
Primer::Beta::Counter.new("aria-hidden": true, **system_arguments)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am directly setting aria-hidden on the counter, because when I nested this inside a span, it caused issues. See #2807 (comment)

}
}

# Trailing action appears to the right of the trailing visual.
Expand Down
12 changes: 12 additions & 0 deletions test/components/primer/beta/button_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ def test_renders_with_leading_visual_icon
assert_selector(".Button .Button-leadingVisual .octicon-alert")
end

def test_renders_with_trailing_visual_counter
render_inline(Primer::Beta::Button.new) do |component|
component.with_trailing_visual_counter(count: 15)
"Button"
end

assert_selector(".Button", text: "Button")
assert_selector('[aria-hidden="true"]', text: "15")
# Parenthesis provides improved screen reader output
assert_selector(".sr-only", text: "(15)")
end

def test_renders_with_leading_visual_svg
render_inline(Primer::Beta::Button.new) do |component|
component.with_leading_visual_svg do
Expand Down