add new link component with optional icon #6
Conversation
jachan
left a comment
jachan
left a comment
There was a problem hiding this comment.
Approved with some comments to consider!
There was a problem hiding this comment.
PEBBLE: if we're not putting a real test here, I'd prefer this file to not exist to not clutter the codebase. Curious how CFA feels though!
There was a problem hiding this comment.
Currently repo has no tests and stubs for all tests - talked over this w Mike/Kat and I'm fine leaving as is
| <% if @icon.present? %> | ||
| <%= image_tag icon_image_path, alt: icon_alt_text, class: "size-[20px]" %> | ||
| <% end %> | ||
| <%= link_to @label, @url %> |
There was a problem hiding this comment.
DUST - We may want to set target=_blank for the link so it opens in a new tab instead of taking the user out of the GetStateCTC flow.
We did so in vita-min on almost all links, and also set rel=noopener nofollow. What do you think?
app/components/link_component.rb
Outdated
| def icon_alt_text | ||
| case @icon | ||
| when "info" | ||
| "info circle" |
There was a problem hiding this comment.
DUST - perhaps info icon is a more descriptive name?
Ticket Link
https://codeforamerica.atlassian.net/browse/TEF-86
Its just a small portion of this ticket, only creating the link.
What was done