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

Support with_leading_visual_svg in Primer::Beta::Button #2215

Merged
merged 13 commits into from
Aug 22, 2023
Merged

Support with_leading_visual_svg in Primer::Beta::Button #2215

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
185490b
Add leading_visual_svg to button component
mjimenez98 Aug 18, 2023
113d4e7
add with_leading_visual_svg example to lookbook
mjimenez98 Aug 18, 2023
33c3f51
use more descriptive labels for buttons
mjimenez98 Aug 18, 2023
2c7ea67
add test
mjimenez98 Aug 18, 2023
f9592c4
Generating component snapshots
mjimenez98 Aug 18, 2023
89904f9
fix ids for buttons in preview
mjimenez98 Aug 18, 2023
65bb542
Merge branch 'mj98-pri-bls' of https://github.com/primer/view_compone…
mjimenez98 Aug 18, 2023
322b533
add changeset
mjimenez98 Aug 18, 2023
e6dd38f
create new preview for button with leading svg icon
mjimenez98 Aug 21, 2023
b7aedb4
fill leading svg icon with current color for button and action_list
mjimenez98 Aug 21, 2023
41121e8
Generating component snapshots
mjimenez98 Aug 21, 2023
c792800
fix stylelint error -- make currentColor lowercase
mjimenez98 Aug 21, 2023
bb34249
Merge branch 'mj98-pri-bls' of https://github.com/primer/view_compone…
mjimenez98 Aug 21, 2023
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
7 changes: 7 additions & 0 deletions .changeset/plenty-experts-sort.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@primer/view-components': minor
---

Support SVGs as leading visuals in `Button` component

<!-- Changed components: Primer::Beta::Button -->
Binary file modified ...shots/snapshots.test.ts-snapshots/primer/beta/button/leading_visual/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 modified ...shots/snapshots.test.ts-snapshots/primer/beta/button/leading_visual/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.
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 @@ -41,10 +41,15 @@ class Button < Primer::Component
#
# - `leading_visual_icon` for a <%= link_to_component(Primer::Beta::Octicon) %>.
#
# - `leading_visual_svg` to render a SVG.
#
# @param system_arguments [Hash] Same arguments as <%= link_to_component(Primer::Beta::Octicon) %>.
renders_one :leading_visual, types: {
icon: lambda { |**system_arguments|
Primer::Beta::Octicon.new(**system_arguments)
},
svg: lambda { |**system_arguments|
Primer::BaseComponent.new(tag: :svg, width: "16", height: "16", **system_arguments)
}
}

Expand Down
17 changes: 15 additions & 2 deletions previews/primer/beta/button_preview/leading_visual.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,22 @@
scheme: scheme,
size: size,
block: block,
id: id,
id: "button-with-leading-icon-preview",
align_content: align_content
)) do |component| %>
<% component.with_leading_visual_icon(icon: :search) %>
Button
Leading icon button
<% end %>

<%= render(Primer::Beta::Button.new(
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind actually creating a new preview for the SVG version so we can isolate the behavior of each? 🙌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sure! Done in e6dd38f

scheme: scheme,
size: size,
block: block,
id: "button-with-leading-svg-preview",
align_content: align_content
)) do |component| %>
<% component.with_leading_visual_svg do %>
<path d="M8 16a2 2 0 001.985-1.75c.017-.137-.097-.25-.235-.25h-3.5c-.138 0-.252.113-.235.25A2 2 0 008 16z"></path><path fill-rule="evenodd" d="M8 1.5A3.5 3.5 0 004.5 5v2.947c0 .346-.102.683-.294.97l-1.703 2.556a.018.018 0 00-.003.01l.001.006c0 .002.002.004.004.006a.017.017 0 00.006.004l.007.001h10.964l.007-.001a.016.016 0 00.006-.004.016.016 0 00.004-.006l.001-.007a.017.017 0 00-.003-.01l-1.703-2.554a1.75 1.75 0 01-.294-.97V5A3.5 3.5 0 008 1.5zM3 5a5 5 0 0110 0v2.947c0 .05.015.098.042.139l1.703 2.555A1.518 1.518 0 0113.482 13H2.518a1.518 1.518 0 01-1.263-2.36l1.703-2.554A.25.25 0 003 7.947V5z"></path>
<% end %>
Leading SVG button
<% end %>
14 changes: 13 additions & 1 deletion test/components/primer/beta/button_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def test_renders_button_invisible_no_visuals
assert_selector(".Button.Button--invisible-noVisuals")
end

def test_renders_with_leading_visual
def test_renders_with_leading_visual_icon
render_inline(Primer::Beta::Button.new) do |component|
component.with_leading_visual_icon(icon: :alert)
"Button"
Expand All @@ -27,6 +27,18 @@ def test_renders_with_leading_visual
assert_selector(".Button .Button-leadingVisual .octicon-alert")
end

def test_renders_with_leading_visual_svg
render_inline(Primer::Beta::Button.new) do |component|
component.with_leading_visual_svg do
'<path d="M8 16a2 2 0 001.985-1.75c.017-.137-.097-.25-.235-.25h-3.5c-.138 0-.252.113-.235.25A2 2 0 008 16z"></path><path fill-rule="evenodd" d="M8 1.5A3.5 3.5 0 004.5 5v2.947c0 .346-.102.683-.294.97l-1.703 2.556a.018.018 0 00-.003.01l.001.006c0 .002.002.004.004.006a.017.017 0 00.006.004l.007.001h10.964l.007-.001a.016.016 0 00.006-.004.016.016 0 00.004-.006l.001-.007a.017.017 0 00-.003-.01l-1.703-2.554a1.75 1.75 0 01-.294-.97V5A3.5 3.5 0 008 1.5zM3 5a5 5 0 0110 0v2.947c0 .05.015.098.042.139l1.703 2.555A1.518 1.518 0 0113.482 13H2.518a1.518 1.518 0 01-1.263-2.36l1.703-2.554A.25.25 0 003 7.947V5z"></path>'.html_safe
end
"Button"
end

assert_selector(".Button", text: "Button")
assert_selector(".Button .Button-leadingVisual")
end

def test_renders_button_with_tooltip
render_inline(Primer::Beta::Button.new(id: "button-id")) do |component|
component.with_tooltip(text: "Tooltip", type: :description)
Expand Down
Loading