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

Updates Button and ButtonMarketing Lookbook previews with accessible examples #2802

Merged
merged 13 commits into from
Apr 25, 2024
Merged

Updates Button and ButtonMarketing Lookbook previews with accessible examples #2802

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fe97df3
Updates Button playground in lookbook to have proper markup
lindseywild Apr 23, 2024
0bb6207
Create function to add to element if tag is
lindseywild Apr 24, 2024
a5d7448
Removes from being a playground option, adds link / summary stories
lindseywild Apr 24, 2024
0c4b7cd
Updates documentation to include requirements for proper markup
lindseywild Apr 24, 2024
6eafc77
Adds changeset
lindseywild Apr 24, 2024
b6dcf12
Generating component snapshots
lindseywild Apr 24, 2024
95164f7
Updates spacing
lindseywild Apr 24, 2024
9895646
Merge branch 'main' into lw/update-button-previews
lindseywild Apr 24, 2024
1226501
Removes summary example from button group
lindseywild Apr 24, 2024
2fa2e10
Updates spacing
lindseywild Apr 24, 2024
63e6083
Generating component snapshots
lindseywild Apr 24, 2024
c2a4f8f
Re-trigger CI
lindseywild Apr 24, 2024
017fd24
Apply suggestions from code review
lindseywild Apr 24, 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/twenty-drinks-peel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Updates Button and ButtonMarketing docs with markup requirements
Binary file added ...ts/snapshots.test.ts-snapshots/primer/beta/button/summary_as_button/default.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...ts/snapshots.test.ts-snapshots/primer/beta/button/summary_as_button/focused.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions app/components/primer/alpha/button_marketing.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
module Primer
module Alpha
# Use `ButtonMarketing` for actions (e.g. in forms). Use links for destinations, or moving from one page to another.
# @accessibility
# Setting the tag argument to `:a:` requires you to pass in an `href` attribute.
class ButtonMarketing < Primer::Component
DEFAULT_SCHEME = :default
SCHEME_MAPPINGS = {
Expand Down
5 changes: 5 additions & 0 deletions app/components/primer/beta/button.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
module Primer
module Beta
# Use `Button` for actions (e.g. in forms). Use links for destinations, or moving from one page to another.
# @accessibility
# Additional markup is required if setting the `tag` argument to either `:a` or `:summary`.
#
# * `:a:` requires you to pass in an `href` attribute
# * `:summary` requires you to wrap the component in a `<details>` element
class Button < Primer::Component
status :beta

Expand Down
11 changes: 9 additions & 2 deletions previews/primer/alpha/button_marketing_preview.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ class ButtonMarketingPreview < ViewComponent::Preview
# @label Playground
# @param scheme [Symbol] select [default, primary, outline, transparent]
# @param variant [Symbol] select [default, large]
# @param tag [Symbol] select [button, a]
# @param type [Symbol] select [button, submit]
# @param disabled [Boolean]
def playground(tag: :button, type: :button, scheme: :default, variant: :default, disabled: false)
Expand All @@ -17,12 +16,20 @@ def playground(tag: :button, type: :button, scheme: :default, variant: :default,
# @label Default options
# @param scheme [Symbol] select [default, primary, outline, transparent]
# @param variant [Symbol] select [default, large]
# @param tag [Symbol] select [button, a]
# @param type [Symbol] select [button, submit]
def default(tag: :button, type: :button, scheme: :default, variant: :default)
render(Primer::Alpha::ButtonMarketing.new(tag: tag, type: type, scheme: scheme, variant: variant)) { "Default" }
end

# @label Link as button options
# @param scheme [Symbol] select [default, primary, outline, transparent]
# @param variant [Symbol] select [default, large]
# @param type [Symbol] select [button, submit]
# @param href [String]
def link_as_button(tag: :a, href: "#", type: :button, scheme: :default, variant: :default)
render(Primer::Alpha::ButtonMarketing.new(tag: tag, href: href, type: type, scheme: scheme, variant: variant)) { "Default" }
end

# @!group Size Variants
# @snapshot
#
Expand Down
10 changes: 3 additions & 7 deletions previews/primer/beta/button_group_preview.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,22 +63,18 @@ def icon_buttons(size: :medium)
end
end

# @label Button group with all tags
# @label Button group with multiple tags
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 don't know if we'd ever want a summary tag in a button group, so I've removed this example.

# @snapshot
def all_tags
def multiple_tags
render(Primer::Beta::ButtonGroup.new) do |component|
component.with_button(id: "button-1", tag: :button) do |button|
button.with_tooltip(text: "Button Tooltip")
"Button 1"
end
component.with_button(id: "button-2", tag: :a) do |button|
component.with_button(id: "button-2", tag: :a, href: "#") do |button|
button.with_tooltip(text: "Button Tooltip")
"Button 2"
end
component.with_button(id: "button-3", tag: :summary) do |button|
button.with_tooltip(text: "Button Tooltip")
"Button 3"
end
end
end

Expand Down
39 changes: 28 additions & 11 deletions previews/primer/beta/button_preview.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class ButtonPreview < ViewComponent::Preview
# @param disabled toggle
# @param inactive toggle
# @param align_content select [center, start]
# @param tag select [a, summary, button]
Copy link
Contributor

Choose a reason for hiding this comment

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

Since tag is still an option for the component, I think we should at least keep it in the Playground story. This is what we use to test different cases and make sure we haven't broken anything.. I would at least keep it to button, a and maybe there's a way to pass in href if a is selected?

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 can definitely keep it to button and a! There is a way I can dynamically add a # if it is an <a> so I will do that :) Thank you!

# @param label_wrap toggle
def playground(
scheme: :default,
Expand All @@ -42,16 +41,15 @@ def playground(
tag: tag,
disabled: disabled,
inactive: inactive,
label_wrap: label_wrap
)) do |_c|
label_wrap: label_wrap
)) do |_c|
"Button"
end
end

# @label Default
# @param block toggle
# @param disabled toggle
# @param tag select [a, summary, button]
def default(
block: false,
id: "button-preview",
Expand All @@ -73,7 +71,6 @@ def default(
# @label Primary
# @param block toggle
# @param disabled toggle
# @param tag select [a, summary, button]
def primary(
id: "button-preview",
block: false,
Expand All @@ -95,7 +92,6 @@ def primary(
# @label Danger
# @param block toggle
# @param disabled toggle
# @param tag select [a, summary, button]
def danger(
id: "button-preview",
block: false,
Expand All @@ -117,7 +113,6 @@ def danger(
# @label Invisible
# @param block toggle
# @param disabled toggle
# @param tag select [a, summary, button]
def invisible(
id: "button-preview",
block: false,
Expand Down Expand Up @@ -145,7 +140,6 @@ def invisible_all_visuals
# @label Link
# @param block toggle
# @param disabled toggle
# @param tag select [a, summary, button]
# @snapshot
def link(
id: "button-preview",
Expand Down Expand Up @@ -178,7 +172,6 @@ def all_schemes(

# @label Full width
# @param disabled toggle
# @param tag select [a, summary, button]
# @snapshot
def full_width(
id: "button-preview",
Expand Down Expand Up @@ -222,6 +215,7 @@ def label_wrap(
# @param size select [small, medium]
# @param block toggle
# @param align_content select [center, start]
# @param href
# @snapshot
def link_as_button(
scheme: :default,
Expand All @@ -245,20 +239,43 @@ def link_as_button(
end
end

# @label Summary as button
# @param scheme select [default, primary, danger, invisible, link]
# @param size select [small, medium]
# @param block toggle
# @param align_content select [center, start]
# @snapshot
def summary_as_button(
scheme: :default,
size: :medium,
block: false,
id: "button-preview",
align_content: :center,
tag: :summary
)
render_with_template(locals: {
scheme: scheme,
size: size,
block: block,
id: id,
align_content: align_content,
tag: tag
})
end

# @label Trailing visual
# @param scheme select [default, primary, danger, invisible, link]
# @param size select [small, medium]
# @param block toggle
# @param align_content select [center, start]
# @param tag select [a, summary, button]
# @snapshot
def trailing_visual(
scheme: :default,
size: :medium,
block: false,
id: "button-preview",
align_content: :center,
tag: :a
tag: :button
)
render_with_template(locals: {
scheme: scheme,
Expand Down
14 changes: 14 additions & 0 deletions previews/primer/beta/button_preview/summary_as_button.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<details>
<%= render(Primer::Beta::Button.new(
scheme: scheme,
size: size,
block: block,
id: id,
align_content: align_content,
tag: tag
)) do %>
Button
<% end %>
<p>A wrapping `details` tag is required when using the button with the `:summary` tag</p>
</details>

30 changes: 28 additions & 2 deletions static/info_arch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2514,7 +2514,7 @@
{
"fully_qualified_name": "Primer::Alpha::ButtonMarketing",
"description": "Use `ButtonMarketing` for actions (e.g. in forms). Use links for destinations, or moving from one page to another.",
"accessibility_docs": null,
"accessibility_docs": "Setting the tag argument to `:a:` requires you to pass in an `href` attribute.",
"is_form_component": false,
"is_published": true,
"requires_js": false,
Expand Down Expand Up @@ -2595,6 +2595,19 @@
]
}
},
{
"preview_path": "primer/alpha/button_marketing/link_as_button",
"name": "link_as_button",
"snapshot": "false",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
},
{
"preview_path": "primer/alpha/button_marketing/sizes_default",
"name": "sizes_default",
Expand Down Expand Up @@ -10928,7 +10941,7 @@
{
"fully_qualified_name": "Primer::Beta::Button",
"description": "Use `Button` for actions (e.g. in forms). Use links for destinations, or moving from one page to another.",
"accessibility_docs": null,
"accessibility_docs": "Additional markup is required if setting the `tag` argument to either `:a` or `:summary`.\n\n* `:a:` requires you to pass in an `href` attribute\n* `:summary` requires you to wrap the component in a `<details>` element",
"is_form_component": false,
"is_published": true,
"requires_js": false,
Expand Down Expand Up @@ -11203,6 +11216,19 @@
]
}
},
{
"preview_path": "primer/beta/button/summary_as_button",
"name": "summary_as_button",
"snapshot": "true",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
},
{
"preview_path": "primer/beta/button/trailing_visual",
"name": "trailing_visual",
Expand Down
26 changes: 26 additions & 0 deletions static/previews.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2163,6 +2163,19 @@
]
}
},
{
"preview_path": "primer/beta/button/summary_as_button",
"name": "summary_as_button",
"snapshot": "true",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
},
{
"preview_path": "primer/beta/button/trailing_visual",
"name": "trailing_visual",
Expand Down Expand Up @@ -2400,6 +2413,19 @@
]
}
},
{
"preview_path": "primer/alpha/button_marketing/link_as_button",
"name": "link_as_button",
"snapshot": "false",
"skip_rules": {
"wont_fix": [
"region"
],
"will_fix": [
"color-contrast"
]
}
},
{
"preview_path": "primer/alpha/button_marketing/sizes_default",
"name": "sizes_default",
Expand Down
Loading