Add support for trailing and leading visual icons in Primer::Beta::Link (second try) (#3041)

HDinger · camertron · web-flow · commit 14dd0d3c4cbf · 2024-11-22T11:08:59.000-08:00
Co-authored-by: Cameron Dutro &lt;camertron@gmail.com&gt;
Co-authored-by: camertron &lt;camertron@users.noreply.github.com&gt;
diff --git a/.changeset/odd-dots-laugh.md b/.changeset/odd-dots-laugh.md
@@ -0,0 +1,5 @@
+---
+'@primer/view-components': minor
+---
+
+Support leading and trailing icons for Links
diff --git a/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/breadcrumbs/with_beta_truncate/focused.png b/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/breadcrumbs/with_beta_truncate/focused.png
diff --git a/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/breadcrumbs/with_deprecated_truncate/default.png b/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/breadcrumbs/with_deprecated_truncate/default.png
diff --git a/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/breadcrumbs/with_deprecated_truncate/focused.png b/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/breadcrumbs/with_deprecated_truncate/focused.png
diff --git a/app/components/primer/base_component.rb b/app/components/primer/base_component.rb
@@ -151,13 +151,16 @@ class BaseComponent < Primer::Component
     # | :- | :- | :- |
     # | classes | String | CSS class name value to be concatenated with generated Primer CSS classes. |
     # | test_selector | String | Adds `data-test-selector='given value'` in non-Production environments for testing purposes. |
+    # | trim | Boolean | Calls `strip` on the content to remove trailing and leading white spaces. |
     def initialize(tag:, classes: nil, **system_arguments)
       @tag = tag
 
       @system_arguments = validate_arguments(tag: tag, **system_arguments)
 
       @result = Primer::Classify.call(**@system_arguments.merge(classes: classes))
 
+      @trim = !!@system_arguments.delete(:trim)
+
       @system_arguments[:"data-view-component"] = true
       # Filter out Primer keys so they don't get assigned as HTML attributes
       @content_tag_args = add_test_selector(@system_arguments).except(*Primer::Classify::Utilities::UTILITIES.keys)
@@ -167,7 +170,7 @@ def call
       if SELF_CLOSING_TAGS.include?(@tag)
         tag(@tag, @content_tag_args.merge(@result))
       else
-        content_tag(@tag, content, @content_tag_args.merge(@result))
+        content_tag(@tag, @trim ? trimmed_content : content, @content_tag_args.merge(@result))
       end
     end
   end
diff --git a/app/components/primer/beta/button.rb b/app/components/primer/beta/button.rb
@@ -159,17 +159,6 @@ def before_render
           "Button--invisible-noVisuals"
         )
       end
-
-      def trimmed_content
-        return if content.blank?
-
-        trimmed_content = content.strip
-
-        return trimmed_content unless content.html_safe?
-
-        # strip unsets `html_safe`, so we have to set it back again to guarantee that HTML blocks won't break
-        trimmed_content.html_safe # rubocop:disable Rails/OutputSafety
-      end
     end
   end
 end
diff --git a/app/components/primer/beta/link.html.erb b/app/components/primer/beta/link.html.erb
@@ -0,0 +1,14 @@
+<%= render Primer::ConditionalWrapper.new(condition: tooltip?, trim: true, tag: :span, position: :relative) do %>
+  <%= render(Primer::BaseComponent.new(trim: true, **@system_arguments)) do %>
+    <%= render(Primer::BaseComponent.new(tag: :span, classes: "Link-content", trim: true)) do %>
+      <% if leading_visual %>
+        <%= leading_visual %>
+      <% end %>
+      <%= content %>
+      <% if trailing_visual %>
+        <%= trailing_visual %>
+      <% end %>
+    <% end %>
+  <% end %>
+  <%= tooltip if tooltip? %>
+<% end %>
diff --git a/app/components/primer/beta/link.pcss b/app/components/primer/beta/link.pcss
@@ -67,3 +67,12 @@
     color: inherit !important;
   }
 }
+
+.Link-content {
+  display: inline-flex;
+  align-items: center;
+  /* stylelint-disable-next-line primer/typography */
+  line-height: normal;
+  gap: var(--base-size-4);
+  text-decoration: inherit;
+}
diff --git a/app/components/primer/beta/link.rb b/app/components/primer/beta/link.rb
@@ -30,6 +30,32 @@ class Link < Primer::Component
         Primer::Alpha::Tooltip.new(**system_arguments)
       }
 
+      # Leading visuals appear to the left of the link text.
+      #
+      # Use:
+      #
+      # - `leading_visual_icon` which accepts the arguments accepted by <%= link_to_component(Primer::Beta::Octicon) %>.
+      #
+      # @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)
+        }
+      }
+
+      # Trailing visuals appear to the right of the link text.
+      #
+      # Use:
+      #
+      # - `trailing_visual_icon` which accepts the arguments accepted by <%= link_to_component(Primer::Beta::Octicon) %>.
+      #
+      # @param system_arguments [Hash] Same arguments as <%= link_to_component(Primer::Beta::Octicon) %>.
+      renders_one :trailing_visual, types: {
+        icon: lambda { |**system_arguments|
+          Primer::Beta::Octicon.new(**system_arguments)
+        }
+      }
+
       # @param href [String] URL to be used for the Link. Required. If the requirements are not met an error will be raised in non production environments. In production, an empty link element will be rendered.
       # @param scheme [Symbol] <%= one_of(Primer::Beta::Link::SCHEME_MAPPINGS.keys) %>
       # @param muted [Boolean] Uses light gray for Link color, and blue on hover.
@@ -54,20 +80,6 @@ def initialize(href: nil, scheme: DEFAULT_SCHEME, muted: false, underline: false
       def before_render
         raise ArgumentError, "href is required" if @system_arguments[:href].nil? && !Rails.env.production?
       end
-
-      def call
-        if tooltip.present?
-          render Primer::BaseComponent.new(tag: :span, position: :relative) do
-            render(Primer::BaseComponent.new(**@system_arguments)) do
-              content
-            end.to_s + tooltip.to_s
-          end
-        else
-          render(Primer::BaseComponent.new(**@system_arguments)) do
-            content
-          end
-        end
-      end
     end
   end
 end
diff --git a/app/components/primer/button_component.rb b/app/components/primer/button_component.rb
@@ -111,16 +111,5 @@ def initialize(
     def link?
       @scheme == LINK_SCHEME
     end
-
-    def trimmed_content
-      return if content.blank?
-
-      trimmed_content = content.strip
-
-      return trimmed_content unless content.html_safe?
-
-      # strip unsets `html_safe`, so we have to set it back again to guarantee that HTML blocks won't break
-      trimmed_content.html_safe # rubocop:disable Rails/OutputSafety
-    end
   end
 end
diff --git a/app/components/primer/component.rb b/app/components/primer/component.rb
@@ -148,5 +148,12 @@ def shouldnt_raise_error?
     def should_raise_aria_error?
       !Rails.env.production? && raise_on_invalid_aria? && !ENV["PRIMER_WARNINGS_DISABLED"]
     end
+
+    def trimmed_content
+      return content unless content.present?
+
+      # strip unsets `html_safe`, so we have to set it back again to guarantee that HTML blocks won't break
+      content.html_safe? ? content.strip.html_safe : content.strip # rubocop:disable Rails/OutputSafety
+    end
   end
 end
diff --git a/app/components/primer/conditional_wrapper.rb b/app/components/primer/conditional_wrapper.rb
@@ -10,12 +10,15 @@ class ConditionalWrapper < Primer::Component
     def initialize(condition:, **base_component_arguments)
       @condition = condition
       @base_component_arguments = base_component_arguments
+      @trim = !!@base_component_arguments.delete(:trim)
     end
 
     def call
-      return content unless @condition
+      unless @condition
+        return @trim ? trimmed_content : content
+      end
 
-      BaseComponent.new(**@base_component_arguments).render_in(self) { content }
+      BaseComponent.new(trim: @trim, **@base_component_arguments).render_in(self) { content }
     end
   end
 end
diff --git a/previews/primer/beta/link_preview.rb b/previews/primer/beta/link_preview.rb
@@ -9,8 +9,14 @@ class LinkPreview < ViewComponent::Preview
       # @param underline [Boolean]
       # @param muted [Boolean]
       # @param scheme [Symbol] select [default, primary, secondary]
-      def playground(scheme: :default, muted: false, underline: true)
-        render(Primer::Beta::Link.new(href: "#", scheme: scheme, muted: muted, underline: underline)) { "This is a link!" }
+      # @param leading_visual_icon [Symbol] octicon
+      # @param trailing_visual_icon [Symbol] octicon
+      def playground(scheme: :default, muted: false, underline: true, leading_visual_icon: nil, trailing_visual_icon: nil)
+        render(Primer::Beta::Link.new(href: "#", scheme: scheme, muted: muted, underline: underline)) do |link|
+          link.with_leading_visual_icon(icon: leading_visual_icon) if leading_visual_icon && leading_visual_icon != :none
+          link.with_trailing_visual_icon(icon: trailing_visual_icon) if trailing_visual_icon && trailing_visual_icon != :none
+          "This is a link!"
+        end
       end
 
       # @label Default Options
@@ -66,6 +72,22 @@ def color_scheme_secondary_muted
         render(Primer::Beta::Link.new(href: "#", scheme: :secondary, muted: true)) { "This is a muted secondary link color." }
       end
       # @!endgroup
+
+      # @label With leading icon
+      def with_leading_icon
+        render(Primer::Beta::Link.new(href: "#")) do |component|
+          component.with_leading_visual_icon(icon: :"mark-github")
+          "Link with leading icon"
+        end
+      end
+
+      # @label With trailing icon
+      def with_trailing_icon
+        render(Primer::Beta::Link.new(href: "#")) do |component|
+          component.with_trailing_visual_icon(icon: :"link-external")
+          "Link with trailing icon"
+        end
+      end
     end
   end
 end
diff --git a/test/components/beta/link_test.rb b/test/components/beta/link_test.rb
@@ -15,7 +15,7 @@ def test_renders_content_and_not_muted_link
   def test_renders_no_additional_whitespace
     result = render_inline(Primer::Beta::Link.new(href: "http://joe-jonas-shirtless.com")) { "content" }
 
-    assert_match(%r{^<a[^>]+>content</a>$}, result.to_s)
+    assert_match(%r{^<a[^>]+><span[^>]+>content</span></a>$}, result.to_s)
   end
 
   def test_renders_without_trailing_newline
@@ -92,4 +92,26 @@ def test_renders_with_tooltip_sibling
 
     assert_selector("a[href='http://google.com'] + tool-tip", text: "Tooltip text", visible: false)
   end
+
+  def test_renders_leading_visual_icon
+    render_inline(Primer::Beta::Link.new(href: "http://google.com")) do |component|
+      component.with_leading_visual_icon(icon: "plus")
+      "content"
+    end
+
+    assert_selector("a[href='http://google.com']")
+    assert_selector(".octicon-plus")
+  end
+
+  def test_renders_trailing_visual_icon
+    render_inline(Primer::Beta::Link.new(href: "http://google.com")) do |component|
+      component.with_leading_visual_icon(icon: "plus")
+      component.with_trailing_visual_icon(icon: "alert")
+      "content"
+    end
+
+    assert_selector("a[href='http://google.com']")
+    assert_selector("a svg:first-child.octicon-plus")
+    assert_selector("a svg:nth-child(2).octicon-alert")
+  end
 end

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@primer/view-components': minor
 +---
++
 +Support leading and trailing icons for Links
Original file line number	Diff line number	Diff line change
`@@ -67,3 +67,12 @@`
`67`	`67`	`color: inherit !important;`
`68`	`68`	`}`
`69`	`69`	`}`
	`70`	`+`
	`71`	`+.Link-content {`
	`72`	`+ display: inline-flex;`
	`73`	`+ align-items: center;`
	`74`	`+ /* stylelint-disable-next-line primer/typography */`
	`75`	`+ line-height: normal;`
	`76`	`+ gap: var(--base-size-4);`
	`77`	`+ text-decoration: inherit;`
	`78`	`+}`