Add trailing visuals to the text field #3237

bsatarnejad · 2024-12-10T08:45:41Z

What are you trying to accomplish?

Like leading visuals in a text-field, add trailing visuals to it. As explained here.

Screenshots

Closes #3218

Risk Assessment

Low risk the change is small, highly observable, and easily rolled back.
Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

Like leading visuals in text field, we add a section for trailing visuals:

Icon
Text
Label
Counter

Merge checklist

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

changeset-bot · 2024-12-10T08:45:45Z

🦋 Changeset detected

Latest commit: 5f4955c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

camertron · 2024-12-17T21:42:13Z

Hey @bsatarnejad, were you planning on implementing the other kinds of trailing visuals we talked about in #3218 (comment) ?

bsatarnejad · 2024-12-18T08:30:35Z

Hey @bsatarnejad, were you planning on implementing the other kinds of trailing visuals we talked about in #3218 (comment) ?

Hi @camertron Yes of course.
The trailing icon has been successfully implemented, and the trailing text is currently being worked on. The remaining items will be addressed in due course.

…ling visuals including icon, text, label and counter

bsatarnejad · 2024-12-19T12:32:23Z

Hi @camertron
As you mentioned here, four possible forms of trailing visual for an input text field are implemented: icon, label, text, and counter.

In the React implementation of this component, I noticed an issue: when there’s a long trailing text, it takes up all the available space, leaving no room for the input field itself.
https://github.com/user-attachments/assets/b33917c8-053c-404d-9bcb-374dc4b97cd4

The solution I’ve implemented for a long trailing text is to handle it by truncating it with ellipsis.
Therefore, I set the trailing text width to 25% of the parent container’s width, ensuring that the input field still has enough space. If the trailing text exceeds this width, it gets truncated with ellipsis.

I’d love to hear your thoughts on this solution!

github-actions · 2024-12-19T12:32:39Z

Uh oh! @bsatarnejad, the image you shared is missing helpful alt text. Check #3237 (comment).

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

camertron

Looking good 😄

camertron · 2024-12-20T18:52:53Z

app/lib/primer/forms/text_field.rb

+      def trailing_visual_render
+        visual = @input.trailing_visual
+        return unless visual
+
+        content = ActiveSupport::SafeBuffer.new # Use SafeBuffer for safe HTML concatenation
+
+        # Render icon if specified
+        content << render(Primer::Beta::Octicon.new(icon: visual[:icon], classes: "FormControl-input-trailingVisualIcon")) if visual[:icon]
+
+        # Render label if specified
+        content << render(Primer::Beta::Label.new(classes: "FormControl-input-trailingVisualLabel")) { visual[:label] } if visual[:label]
+
+        # Render counter if specified
+        content << render(Primer::Beta::Counter.new(count: visual[:counter], classes: "FormControl-input-trailingVisualCounter")) if visual[:counter]
+
+        # Render text if specified
+        content << content_tag(:span, visual[:text], class: "FormControl-input-trailingVisualText") if visual[:text]
+
+        # Wrap in the trailing visual container
+        content_tag(:span, content, class: "FormControl-input-trailingVisualWrap")
+      end


Hmm, I like where you're going with this, but I have a few suggestions.

The truncation idea is cool 😎 I actually think that's better than what the React version does... although in React, you can pass in whatever custom component you want, so 🤷 . In any case, let's use Primer::Beta::Truncate for this.

I would prefer rendering to be done in the template.

Let's try to allow the caller to pass in system arguments to Label, Octicon, etc.

I don't think it makes sense to allow adding eg. both a label and a counter, so let's limit the trailing visuals to a single one... unless you have a use-case for multiple trailing visuals?

Here's a suggestion that incorporates all these ideas (see my other comment in the template as well)

Suggested change

def trailing_visual_render

visual = @input.trailing_visual

return unless visual

content = ActiveSupport::SafeBuffer.new # Use SafeBuffer for safe HTML concatenation

# Render icon if specified

content << render(Primer::Beta::Octicon.new(icon: visual[:icon], classes: "FormControl-input-trailingVisualIcon")) if visual[:icon]

# Render label if specified

content << render(Primer::Beta::Label.new(classes: "FormControl-input-trailingVisualLabel")) { visual[:label] } if visual[:label]

# Render counter if specified

content << render(Primer::Beta::Counter.new(count: visual[:counter], classes: "FormControl-input-trailingVisualCounter")) if visual[:counter]

# Render text if specified

content << content_tag(:span, visual[:text], class: "FormControl-input-trailingVisualText") if visual[:text]

# Wrap in the trailing visual container

content_tag(:span, content, class: "FormControl-input-trailingVisualWrap")

end

def trailing_visual_component

return @trailing_visual_component if defined?(@trailing_visual_component)

visual = @input.trailing_visual

# Render icon if specified

@trailing_visual_component = if (icon_arguments = visual[:icon])

icon_arguments[:classes] = class_names(

icon_arguments.delete(:classes),

"FormControl-input-trailingVisualIcon"

)

Primer::Beta::Octicon.new(**icon_arguments)

elsif (label_arguments = visual[:label])

# Render label if specified

label_arguments[:classes] = class_names(

label_arguments.delete(:classes),

"FormControl-input-trailingVisualLabel"

)

text = label_arguments.delete(:text)

Primer::Beta::Label.new(**label_arguments).with_content(text)

elsif (counter_arguments = visual[:counter])

# Render counter if specified

counter_arguments[:classes] = class_names(

counter_arguments.delete(:classes),

"FormControl-input-trailingVisualCounter"

)

Primer::Beta::Counter.new(**counter_arguments))

elsif (truncate_arguments = visual[:text])

# Render text if specified

text = truncate_arguments.delete(:text)

Primer::Beta::Truncate.new(**truncate_arguments).with_content(text)

end

end

camertron · 2024-12-20T18:54:37Z

app/lib/primer/forms/text_field.html.erb

+    <% if @input.trailing_visual %>
+      <%= trailing_visual_render %>
+    <% end %>


This is meant to be combined with my suggestion that introduces the trailing_visual_component method:

Suggested change

<% if @input.trailing_visual %>

<%= trailing_visual_render %>

<% end %>

<% if (component = @input.trailing_visual_component) %>

<div class="FormControl-input-trailingVisualWrap">

<%= render(component) %>

</div>

<% end %>

bsatarnejad requested review from a team as code owners December 10, 2024 08:45

bsatarnejad requested review from langermank and removed request for a team December 10, 2024 08:45

lesliecdubs requested a review from camertron December 10, 2024 17:03

bsatarnejad and others added 2 commits December 17, 2024 07:49

add trailing visual to the primer text field

e1720eb

Create changeset

5396bf2

bsatarnejad force-pushed the 59860-add-trailing-unit-for-input-fields-in-datepicker-and-lag-field branch from d650220 to 5396bf2 Compare December 17, 2024 06:49

camertron mentioned this pull request Dec 17, 2024

59860 add trailing unit for input fields in datepicker and lag field #3246

Draft

10 tasks

camertron had a problem deploying to preview December 17, 2024 18:26 — with GitHub Actions Error

bsatarnejad added 4 commits December 18, 2024 12:00

add trailing text to the primer text field

2630356

remove duplicated condition

caaa979

change the text input field component to have the possibility of trai…

5a81e1a

…ling visuals including icon, text, label and counter

add test for trailing visuals

5f4955c

camertron requested changes Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add trailing visuals to the text field #3237

Add trailing visuals to the text field #3237

bsatarnejad commented Dec 10, 2024 •

edited

Loading

changeset-bot bot commented Dec 10, 2024 •

edited

Loading

camertron commented Dec 17, 2024

bsatarnejad commented Dec 18, 2024

bsatarnejad commented Dec 19, 2024 •

edited

Loading

github-actions bot commented Dec 19, 2024

camertron left a comment

camertron Dec 20, 2024

camertron Dec 20, 2024

Add trailing visuals to the text field #3237

Are you sure you want to change the base?

Add trailing visuals to the text field #3237

Conversation

bsatarnejad commented Dec 10, 2024 • edited Loading

What are you trying to accomplish?

Screenshots

Risk Assessment

What approach did you choose and why?

Merge checklist

changeset-bot bot commented Dec 10, 2024 • edited Loading

🦋 Changeset detected

camertron commented Dec 17, 2024

bsatarnejad commented Dec 18, 2024

bsatarnejad commented Dec 19, 2024 • edited Loading

github-actions bot commented Dec 19, 2024

camertron left a comment

Choose a reason for hiding this comment

camertron Dec 20, 2024

Choose a reason for hiding this comment

camertron Dec 20, 2024

Choose a reason for hiding this comment

bsatarnejad commented Dec 10, 2024 •

edited

Loading

changeset-bot bot commented Dec 10, 2024 •

edited

Loading

bsatarnejad commented Dec 19, 2024 •

edited

Loading