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.

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

Add trailing visuals to the text field #3237

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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/new-students-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Add trailing visuals to the text field
68 changes: 67 additions & 1 deletion app/components/primer/alpha/text_field.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@
** .FormControl
** ├─ .FormControl-label
** │ ├─ .FormControl-input-wrap
** │ │ ├─ .FormControl-input-trailingVisualWrap
** │ │ │ ├─ .FormControl-input-trailingVisual
** │ │ ├─ .FormControl-input-leadingVisualWrap
** │ │ │ ├─ .FormControl-input-leadingVisual
** │ │ ├─ .FormControl-input
Expand Down Expand Up @@ -253,6 +255,28 @@
}
}

& .FormControl-input-trailingVisualWrap {
position: absolute;
top: var(--base-size-8);
right: var(--base-size-8);
display: flex;
height: var(--base-size-16);
align-items: center;
gap: var(--base-size-4);
color: var(--fgColor-muted);
pointer-events: none;
&:has( .FormControl-input-trailingVisualText) {
width: auto;
max-width: 25%;
padding-left: var(--base-size-8);

& .FormControl-input-trailingVisualText {
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}
}
}
/* TODO: replace with new Button component */
& .FormControl-input-trailingAction {
position: absolute;
Expand Down Expand Up @@ -333,10 +357,29 @@
}
}

/* if trailingVisual is present */

/*
┌──────────────────┬──32px──┐
╎ ┌──────────────┐ ┌────┐ ╎
╎ 24px 24px ╎
╎ └──────────────┘ └────┘ ╎
└──────────────────┴────────┘
*/

&.FormControl-input-wrap--trailingVisual {
& .FormControl-input {
padding-inline-end: calc(var(--control-medium-paddingInline-condensed) + var(--base-size-16) + var(--control-medium-gap));
}
&:has(.FormControl-input-trailingVisualText) .FormControl-input {
padding-inline-end: 25%
}
}

/*
┌──────────────────┬──32px──┐
╎ ┌──────────────┐ ┌────┐ ╎
╎ 24px 24px
╎ 24px 24px ╎
╎ └──────────────┘ └────┘ ╎
└──────────────────┴────────┘
*/
Expand Down Expand Up @@ -377,6 +420,10 @@
top: calc(var(--control-medium-paddingInline-condensed) - var(--base-size-2)); /* 6px */
left: calc(var(--control-medium-paddingInline-condensed) - var(--base-size-2)); /* 6px */
}
& .FormControl-input-trailingVisualWrap {
top: calc(var(--control-medium-paddingInline-condensed) - var(--base-size-2)); /* 6px */
right: calc(var(--control-medium-paddingInline-condensed) - var(--base-size-2)); /* 6px */
}

/*
┌──────────────────┬──28px──┐
Expand Down Expand Up @@ -427,6 +474,10 @@
top: var(--control-medium-paddingInline-normal);
left: var(--control-medium-paddingInline-normal);
}
& .FormControl-input-trailingVisualWrap {
top: var(--control-medium-paddingInline-normal);
right: var(--control-medium-paddingInline-normal);
}

/*
┌──36px──┬───12px padding──────┐
Expand All @@ -444,6 +495,21 @@
}
}

&.FormControl-input-wrap--trailingVisual {
& .FormControl-input {
padding-inline-end: calc(var(--control-large-paddingInline-normal) + var(--base-size-16) + var(--control-large-gap));
}
&:has(.FormControl-input-trailingVisualText) .FormControl-input {
padding-inline-end: 25%
}
}

&.FormControl-input-wrap--trailingText {
& .FormControl-input.FormControl-large {
padding-inline-end: 25%;
}
}

/*
┌──────────────────┬──36px──┐
╎ ┌──────────────┐ ┌────┐ ╎
Expand Down
28 changes: 21 additions & 7 deletions app/lib/primer/forms/dsl/text_field_input.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# frozen_string_literal: true

module Primer
module Forms
module Dsl
# :nodoc:
class TextFieldInput < Input
attr_reader(
*%i[
name label show_clear_button leading_visual leading_spinner clear_button_id
name label show_clear_button leading_visual leading_spinner trailing_visual clear_button_id
visually_hide_label inset monospace field_wrap_classes auto_check_src
]
)
Expand All @@ -20,6 +18,7 @@ def initialize(name:, label:, **system_arguments)

@show_clear_button = system_arguments.delete(:show_clear_button)
@leading_visual = system_arguments.delete(:leading_visual)
@trailing_visual = build_trailing_visual(system_arguments.delete(:trailing_visual))
@leading_spinner = !!system_arguments.delete(:leading_spinner)
@clear_button_id = system_arguments.delete(:clear_button_id)
@inset = system_arguments.delete(:inset)
Expand Down Expand Up @@ -48,6 +47,25 @@ def initialize(name:, label:, **system_arguments)
alias inset? inset
alias monospace? monospace

def trailing_visual?
!!@trailing_visual
end

def leading_visual?
!!@leading_visual
end

def build_trailing_visual(trailing_visual)
return nil unless trailing_visual

icon = trailing_visual[:icon]
text = trailing_visual[:text]
counter = trailing_visual[:counter]
label = trailing_visual[:label]

{ icon: icon, text: text, counter: counter, label: label }.compact
end

def to_component
TextField.new(input: self)
end
Expand All @@ -60,10 +78,6 @@ def focusable?
true
end

def leading_visual?
!!@leading_visual
end

def validation_arguments
if auto_check_src.present?
super.merge(
Expand Down
3 changes: 3 additions & 0 deletions app/lib/primer/forms/text_field.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@
<%= render(Primer::Beta::Octicon.new(icon: :"x-circle-fill")) %>
</button>
<% end %>
<% if @input.trailing_visual %>
<%= trailing_visual_render %>
<% end %>
Comment on lines +20 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

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 %>

<% end %>
<% end %>
29 changes: 25 additions & 4 deletions app/lib/primer/forms/text_field.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# frozen_string_literal: true

module Primer
module Forms
# :nodoc:
class TextField < BaseComponent
delegate :builder, :form, to: :@input

Expand All @@ -24,9 +21,9 @@ def initialize(input:)
"FormControl-input-wrap",
INPUT_WRAP_SIZE[input.size],
"FormControl-input-wrap--trailingAction": @input.show_clear_button?,
"FormControl-input-wrap--trailingVisual": @input.trailing_visual?,
"FormControl-input-wrap--leadingVisual": @input.leading_visual?
),

hidden: @input.hidden?
}
end
Expand All @@ -41,6 +38,30 @@ def auto_check_authenticity_token
)
end
end

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
Comment on lines +42 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

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

  1. 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.
  2. I would prefer rendering to be done in the template.
  3. Let's try to allow the caller to pass in system arguments to Label, Octicon, etc.
  4. 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



end
end
end
24 changes: 24 additions & 0 deletions previews/primer/alpha/text_field_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,30 @@ def monospace
render(Primer::Alpha::TextField.new(monospace: true, name: "my-text-field", label: "My text field"))
end

# @label With trailing icon
# @snapshot
def with_trailing_icon
render(Primer::Alpha::TextField.new(trailing_visual: { icon: :search }, name: "my-text-field", label: "My text field"))
end

# @label With trailing text
# @snapshot
def with_trailing_text
render(Primer::Alpha::TextField.new( trailing_visual: { text: "minute" }, name: "my-text-field", label: "My text field"))
end

# @label With trailing counter
# @snapshot
def with_trailing_counter
render(Primer::Alpha::TextField.new( trailing_visual: { counter: 5 }, name: "my-text-field", label: "My text field"))
end

# @label With trailing label
# @snapshot
def with_trailing_label
render(Primer::Alpha::TextField.new( trailing_visual: { label: "Hello" }, name: "my-text-field", label: "My text field"))
end

# @label With leading visual
# @snapshot
def with_leading_visual
Expand Down
32 changes: 32 additions & 0 deletions test/components/alpha/text_field_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,36 @@ def test_enforces_leading_visual_when_spinner_requested

assert_includes error.message, "must also specify a leading visual"
end

def test_renders_a_trailing_visual_icon
render_inline(Primer::Alpha::TextField.new(**@default_params, trailing_visual: { icon: :search }))

assert_selector ".FormControl-input-trailingVisualWrap" do
assert_selector "svg.octicon.octicon-search.FormControl-input-trailingVisualIcon"
end
end

def test_renders_a_trailing_visual_text
render_inline(Primer::Alpha::TextField.new(**@default_params, trailing_visual: { text: 'minute' }))

assert_selector ".FormControl-input-trailingVisualWrap" do
assert_selector ".FormControl-input-trailingVisualText", text: "minute"
end
end

def test_renders_a_trailing_visual_label
render_inline(Primer::Alpha::TextField.new(**@default_params, trailing_visual: { label: 'Hello' }))

assert_selector ".FormControl-input-trailingVisualWrap" do
assert_selector ".FormControl-input-trailingVisualLabel.Label", text: "Hello"
end
end

def test_renders_a_trailing_visual_Counter
render_inline(Primer::Alpha::TextField.new(**@default_params, trailing_visual: { counter: '5' }))

assert_selector ".FormControl-input-trailingVisualWrap" do
assert_selector ".FormControl-input-trailingVisualCounter.Counter", text: "5"
end
end
end