From e1720eb37b6df50f029cda3f86ca144cabbe30de Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Mon, 9 Dec 2024 15:47:04 +0100 Subject: [PATCH 01/10] add trailing visual to the primer text field --- app/components/primer/alpha/text_field.pcss | 54 +++++++++++++++++++- app/lib/primer/forms/dsl/text_field_input.rb | 22 ++++++-- app/lib/primer/forms/text_field.html.erb | 7 +++ app/lib/primer/forms/text_field.rb | 1 + previews/primer/alpha/text_field_preview.rb | 6 +++ 5 files changed, 84 insertions(+), 6 deletions(-) diff --git a/app/components/primer/alpha/text_field.pcss b/app/components/primer/alpha/text_field.pcss index cb46fdb8ae..12d43406e4 100644 --- a/app/components/primer/alpha/text_field.pcss +++ b/app/components/primer/alpha/text_field.pcss @@ -121,6 +121,8 @@ ** .FormControl ** ├─ .FormControl-label ** │ ├─ .FormControl-input-wrap +** │ │ ├─ .FormControl-input-trailingVisualWrap +** │ │ │ ├─ .FormControl-input-trailingVisual ** │ │ ├─ .FormControl-input-leadingVisualWrap ** │ │ │ ├─ .FormControl-input-leadingVisual ** │ │ ├─ .FormControl-input @@ -253,6 +255,23 @@ } } + & .FormControl-input-trailingVisualWrap { + position: absolute; + top: var(--base-size-8); + right: var(--base-size-8); + display: block; + width: var(--base-size-16); + height: var(--base-size-16); + color: var(--fgColor-muted); + pointer-events: none; + + /* octicon */ + & .FormControl-input-trailingVisual { + display: block; + user-select: none; + } + } + /* TODO: replace with new Button component */ & .FormControl-input-trailingAction { position: absolute; @@ -333,10 +352,28 @@ } } + /* 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) + ); /* 32px */ + } + } + /* ┌──────────────────┬──32px──┐ ╎ ┌──────────────┐ ┌────┐ ╎ - ╎ 24px 24px ╎ + ╎ 24px 24px ╎ ╎ └──────────────┘ └────┘ ╎ └──────────────────┴────────┘ */ @@ -377,6 +414,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──┐ @@ -427,6 +468,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──────┐ @@ -443,6 +488,13 @@ ); /* 36px */ } } + &.FormControl-input-wrap--trailingVisual { + & .FormControl-input.FormControl-large { + padding-inline-end: calc( + var(--control-large-paddingInline-normal) + var(--base-size-16) + var(--control-large-gap) + ); /* 36px */ + } + } /* ┌──────────────────┬──36px──┐ diff --git a/app/lib/primer/forms/dsl/text_field_input.rb b/app/lib/primer/forms/dsl/text_field_input.rb index 698189fcc8..e8e8901ead 100644 --- a/app/lib/primer/forms/dsl/text_field_input.rb +++ b/app/lib/primer/forms/dsl/text_field_input.rb @@ -7,7 +7,7 @@ module Dsl 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 ] ) @@ -20,6 +20,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 = 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) @@ -33,6 +34,13 @@ def initialize(name:, label:, **system_arguments) ) end + if @trailing_visual + @trailing_visual[:classes] = class_names( + "FormControl-input-trailingVisual", + @trailing_visual[:classes] + ) + end + if @leading_spinner && !@leading_visual raise ArgumentError, "text fields that request a leading spinner must also specify a leading visual" end @@ -48,6 +56,14 @@ 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 to_component TextField.new(input: self) end @@ -60,10 +76,6 @@ def focusable? true end - def leading_visual? - !!@leading_visual - end - def validation_arguments if auto_check_src.present? super.merge( diff --git a/app/lib/primer/forms/text_field.html.erb b/app/lib/primer/forms/text_field.html.erb index 7b06434e76..a7e99afaee 100644 --- a/app/lib/primer/forms/text_field.html.erb +++ b/app/lib/primer/forms/text_field.html.erb @@ -17,5 +17,12 @@ <%= render(Primer::Beta::Octicon.new(icon: :"x-circle-fill")) %> <% end %> + <% if @input.trailing_visual%> + + <% if @input.trailing_visual %> + <%= render(Primer::Beta::Octicon.new(**@input.trailing_visual, data: { target: "primer-text-field.trailingVisual" })) %> + <% end %> + + <% end %> <% end %> <% end %> diff --git a/app/lib/primer/forms/text_field.rb b/app/lib/primer/forms/text_field.rb index 1840f92434..07b68fb6bf 100644 --- a/app/lib/primer/forms/text_field.rb +++ b/app/lib/primer/forms/text_field.rb @@ -24,6 +24,7 @@ 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? ), diff --git a/previews/primer/alpha/text_field_preview.rb b/previews/primer/alpha/text_field_preview.rb index e077ff0d09..d020cfdd9f 100644 --- a/previews/primer/alpha/text_field_preview.rb +++ b/previews/primer/alpha/text_field_preview.rb @@ -172,6 +172,12 @@ def monospace render(Primer::Alpha::TextField.new(monospace: true, name: "my-text-field", label: "My text field")) end + # @label With trailing visual + # @snapshot + def with_trailing_visual + render(Primer::Alpha::TextField.new(trailing_visual: { icon: :search }, name: "my-text-field", label: "My text field")) + end + # @label With leading visual # @snapshot def with_leading_visual From 5396bf243584da5e60de48d2dc83673323921db1 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad <62008897+bsatarnejad@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:43:39 +0100 Subject: [PATCH 02/10] Create changeset --- .changeset/new-students-change.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/new-students-change.md diff --git a/.changeset/new-students-change.md b/.changeset/new-students-change.md new file mode 100644 index 0000000000..e983d38e24 --- /dev/null +++ b/.changeset/new-students-change.md @@ -0,0 +1,5 @@ +--- +"@primer/view-components": patch +--- + +Add trailing visuals to the text field From 26303565f43bfde3bdfead27dc36fc3fc899b04b Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Wed, 18 Dec 2024 12:00:21 +0100 Subject: [PATCH 03/10] add trailing text to the primer text field --- app/components/primer/alpha/text_field.pcss | 32 ++++++++++++++++++++ app/lib/primer/forms/dsl/text_field_input.rb | 7 ++++- app/lib/primer/forms/text_field.html.erb | 5 +++ app/lib/primer/forms/text_field.rb | 3 +- previews/primer/alpha/text_field_preview.rb | 6 ++++ 5 files changed, 51 insertions(+), 2 deletions(-) diff --git a/app/components/primer/alpha/text_field.pcss b/app/components/primer/alpha/text_field.pcss index 12d43406e4..fe1239e1fd 100644 --- a/app/components/primer/alpha/text_field.pcss +++ b/app/components/primer/alpha/text_field.pcss @@ -271,6 +271,26 @@ user-select: none; } } + & .FormControl-input-trailingTextWrap { + position: absolute; + top: var(--base-size-8); + right: var(--base-size-8); + width: auto; + max-width: 25%; + padding-left: var(--base-size-8); + display: flex; + align-items: center; + height: var(--base-size-16); + color: var(--fgColor-muted); + pointer-events: none; + + /* text */ + & .FormControl-input-trailingText { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + } /* TODO: replace with new Button component */ & .FormControl-input-trailingAction { @@ -370,6 +390,12 @@ } } + &.FormControl-input-wrap--trailingText { + & .FormControl-input { + padding-inline-end: 25% + } + } + /* ┌──────────────────┬──32px──┐ ╎ ┌──────────────┐ ┌────┐ ╎ @@ -496,6 +522,12 @@ } } + &.FormControl-input-wrap--trailingText { + & .FormControl-input.FormControl-large { + padding-inline-end: 25%; + } + } + /* ┌──────────────────┬──36px──┐ ╎ ┌──────────────┐ ┌────┐ ╎ diff --git a/app/lib/primer/forms/dsl/text_field_input.rb b/app/lib/primer/forms/dsl/text_field_input.rb index e8e8901ead..daacdb3e66 100644 --- a/app/lib/primer/forms/dsl/text_field_input.rb +++ b/app/lib/primer/forms/dsl/text_field_input.rb @@ -8,7 +8,7 @@ class TextFieldInput < Input attr_reader( *%i[ 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 + visually_hide_label inset monospace field_wrap_classes auto_check_src trailing_text ] ) @@ -21,6 +21,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 = system_arguments.delete(:trailing_visual) + @trailing_text = system_arguments.delete(:trailing_text) @leading_spinner = !!system_arguments.delete(:leading_spinner) @clear_button_id = system_arguments.delete(:clear_button_id) @inset = system_arguments.delete(:inset) @@ -107,6 +108,10 @@ def validation_message_arguments super end end + + def trailing_text? + !!@trailing_text + end end end end diff --git a/app/lib/primer/forms/text_field.html.erb b/app/lib/primer/forms/text_field.html.erb index a7e99afaee..1ce708432b 100644 --- a/app/lib/primer/forms/text_field.html.erb +++ b/app/lib/primer/forms/text_field.html.erb @@ -21,7 +21,12 @@ <% if @input.trailing_visual %> <%= render(Primer::Beta::Octicon.new(**@input.trailing_visual, data: { target: "primer-text-field.trailingVisual" })) %> + <% end %> + <% end %> + <% if @input.trailing_text %> + + <%= @input.trailing_text %> <% end %> <% end %> diff --git a/app/lib/primer/forms/text_field.rb b/app/lib/primer/forms/text_field.rb index 07b68fb6bf..a56b7b0e2f 100644 --- a/app/lib/primer/forms/text_field.rb +++ b/app/lib/primer/forms/text_field.rb @@ -25,7 +25,8 @@ def initialize(input:) 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? + "FormControl-input-wrap--leadingVisual": @input.leading_visual?, + "FormControl-input-wrap--trailingText": @input.trailing_text? ), hidden: @input.hidden? diff --git a/previews/primer/alpha/text_field_preview.rb b/previews/primer/alpha/text_field_preview.rb index d020cfdd9f..b48b78dfeb 100644 --- a/previews/primer/alpha/text_field_preview.rb +++ b/previews/primer/alpha/text_field_preview.rb @@ -178,6 +178,12 @@ def with_trailing_visual 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_text: "minutes", name: "my-text-field", label: "My text field")) + end + # @label With leading visual # @snapshot def with_leading_visual From caaa979d98ad658841c7c52b9b4db4ea608901d8 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Wed, 18 Dec 2024 12:12:40 +0100 Subject: [PATCH 04/10] remove duplicated condition --- app/lib/primer/forms/text_field.html.erb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/lib/primer/forms/text_field.html.erb b/app/lib/primer/forms/text_field.html.erb index 1ce708432b..5983d2baef 100644 --- a/app/lib/primer/forms/text_field.html.erb +++ b/app/lib/primer/forms/text_field.html.erb @@ -19,10 +19,8 @@ <% end %> <% if @input.trailing_visual%> - <% if @input.trailing_visual %> - <%= render(Primer::Beta::Octicon.new(**@input.trailing_visual, data: { target: "primer-text-field.trailingVisual" })) %> - - <% end %> + <%= render(Primer::Beta::Octicon.new(**@input.trailing_visual, data: { target: "primer-text-field.trailingVisual" })) %> + <% end %> <% if @input.trailing_text %> From 5a81e1a202a08d1579b1a9114554075d7d3d5945 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Wed, 18 Dec 2024 16:00:39 +0100 Subject: [PATCH 05/10] change the text input field component to have the possibility of trailing visuals including icon, text, label and counter --- app/components/primer/alpha/text_field.pcss | 58 +++++++------------- app/lib/primer/forms/dsl/text_field_input.rb | 29 +++++----- app/lib/primer/forms/text_field.html.erb | 11 +--- app/lib/primer/forms/text_field.rb | 31 +++++++++-- previews/primer/alpha/text_field_preview.rb | 18 +++++- 5 files changed, 75 insertions(+), 72 deletions(-) diff --git a/app/components/primer/alpha/text_field.pcss b/app/components/primer/alpha/text_field.pcss index fe1239e1fd..490578458c 100644 --- a/app/components/primer/alpha/text_field.pcss +++ b/app/components/primer/alpha/text_field.pcss @@ -259,39 +259,24 @@ position: absolute; top: var(--base-size-8); right: var(--base-size-8); - display: block; - width: var(--base-size-16); - height: var(--base-size-16); - color: var(--fgColor-muted); - pointer-events: none; - - /* octicon */ - & .FormControl-input-trailingVisual { - display: block; - user-select: none; - } - } - & .FormControl-input-trailingTextWrap { - position: absolute; - top: var(--base-size-8); - right: var(--base-size-8); - width: auto; - max-width: 25%; - padding-left: var(--base-size-8); display: flex; - align-items: center; height: var(--base-size-16); + align-items: center; + gap: var(--base-size-4); color: var(--fgColor-muted); pointer-events: none; - - /* text */ - & .FormControl-input-trailingText { - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; + &:has( .FormControl-input-trailingText) { + width: auto; + max-width: 25%; + padding-left: var(--base-size-8); + + & .FormControl-input-trailingText { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } } } - /* TODO: replace with new Button component */ & .FormControl-input-trailingAction { position: absolute; @@ -384,14 +369,9 @@ &.FormControl-input-wrap--trailingVisual { & .FormControl-input { - padding-inline-end: calc( - var(--control-medium-paddingInline-condensed) + var(--base-size-16) + var(--control-medium-gap) - ); /* 32px */ + padding-inline-end: calc(var(--control-medium-paddingInline-condensed) + var(--base-size-16) + var(--control-medium-gap)); } - } - - &.FormControl-input-wrap--trailingText { - & .FormControl-input { + &:has(.FormControl-input-trailingText) .FormControl-input { padding-inline-end: 25% } } @@ -514,11 +494,13 @@ ); /* 36px */ } } + &.FormControl-input-wrap--trailingVisual { - & .FormControl-input.FormControl-large { - padding-inline-end: calc( - var(--control-large-paddingInline-normal) + var(--base-size-16) + var(--control-large-gap) - ); /* 36px */ + & .FormControl-input { + padding-inline-end: calc(var(--control-large-paddingInline-normal) + var(--base-size-16) + var(--control-large-gap)); + } + &:has(.FormControl-input-trailingText) .FormControl-input { + padding-inline-end: 25% } } diff --git a/app/lib/primer/forms/dsl/text_field_input.rb b/app/lib/primer/forms/dsl/text_field_input.rb index daacdb3e66..9aa4dd23f2 100644 --- a/app/lib/primer/forms/dsl/text_field_input.rb +++ b/app/lib/primer/forms/dsl/text_field_input.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - module Primer module Forms module Dsl @@ -8,7 +6,7 @@ class TextFieldInput < Input attr_reader( *%i[ 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 trailing_text + visually_hide_label inset monospace field_wrap_classes auto_check_src ] ) @@ -20,8 +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 = system_arguments.delete(:trailing_visual) - @trailing_text = system_arguments.delete(:trailing_text) + @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) @@ -35,13 +32,6 @@ def initialize(name:, label:, **system_arguments) ) end - if @trailing_visual - @trailing_visual[:classes] = class_names( - "FormControl-input-trailingVisual", - @trailing_visual[:classes] - ) - end - if @leading_spinner && !@leading_visual raise ArgumentError, "text fields that request a leading spinner must also specify a leading visual" end @@ -65,6 +55,17 @@ 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 @@ -108,10 +109,6 @@ def validation_message_arguments super end end - - def trailing_text? - !!@trailing_text - end end end end diff --git a/app/lib/primer/forms/text_field.html.erb b/app/lib/primer/forms/text_field.html.erb index 5983d2baef..207ad1973e 100644 --- a/app/lib/primer/forms/text_field.html.erb +++ b/app/lib/primer/forms/text_field.html.erb @@ -17,15 +17,8 @@ <%= render(Primer::Beta::Octicon.new(icon: :"x-circle-fill")) %> <% end %> - <% if @input.trailing_visual%> - - <%= render(Primer::Beta::Octicon.new(**@input.trailing_visual, data: { target: "primer-text-field.trailingVisual" })) %> - - <% end %> - <% if @input.trailing_text %> - - <%= @input.trailing_text %> - + <% if @input.trailing_visual %> + <%= trailing_visual_render %> <% end %> <% end %> <% end %> diff --git a/app/lib/primer/forms/text_field.rb b/app/lib/primer/forms/text_field.rb index a56b7b0e2f..9b11670d51 100644 --- a/app/lib/primer/forms/text_field.rb +++ b/app/lib/primer/forms/text_field.rb @@ -1,8 +1,5 @@ -# frozen_string_literal: true - module Primer module Forms - # :nodoc: class TextField < BaseComponent delegate :builder, :form, to: :@input @@ -25,10 +22,8 @@ def initialize(input:) 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?, - "FormControl-input-wrap--trailingText": @input.trailing_text? + "FormControl-input-wrap--leadingVisual": @input.leading_visual? ), - hidden: @input.hidden? } end @@ -43,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])) if visual[:icon] + + # Render label if specified + content << render(Primer::Beta::Label.new()) { visual[:label] } if visual[:label] + + # Render counter if specified + content << render(Primer::Beta::Counter.new(count: visual[:counter])) if visual[:counter] + + # Render text if specified + content << content_tag(:span, visual[:text], class: "FormControl-input-trailingText") if visual[:text] + + # Wrap in the trailing visual container + content_tag(:span, content, class: "FormControl-input-trailingVisualWrap") + end + + end end end diff --git a/previews/primer/alpha/text_field_preview.rb b/previews/primer/alpha/text_field_preview.rb index b48b78dfeb..e280a619aa 100644 --- a/previews/primer/alpha/text_field_preview.rb +++ b/previews/primer/alpha/text_field_preview.rb @@ -172,16 +172,28 @@ def monospace render(Primer::Alpha::TextField.new(monospace: true, name: "my-text-field", label: "My text field")) end - # @label With trailing visual + # @label With trailing icon # @snapshot - def with_trailing_visual + 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_text: "minutes", name: "my-text-field", label: "My text field")) + render(Primer::Alpha::TextField.new( trailing_visual: { text: "Foobarbaz" }, 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 From 5f4955c7d8c0182b8e9903dc285a05e570c01e5a Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Thu, 19 Dec 2024 10:56:02 +0100 Subject: [PATCH 06/10] add test for trailing visuals --- app/components/primer/alpha/text_field.pcss | 8 +++--- app/lib/primer/forms/text_field.rb | 8 +++--- previews/primer/alpha/text_field_preview.rb | 2 +- test/components/alpha/text_field_test.rb | 32 +++++++++++++++++++++ 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/app/components/primer/alpha/text_field.pcss b/app/components/primer/alpha/text_field.pcss index 490578458c..94707ab81b 100644 --- a/app/components/primer/alpha/text_field.pcss +++ b/app/components/primer/alpha/text_field.pcss @@ -265,12 +265,12 @@ gap: var(--base-size-4); color: var(--fgColor-muted); pointer-events: none; - &:has( .FormControl-input-trailingText) { + &:has( .FormControl-input-trailingVisualText) { width: auto; max-width: 25%; padding-left: var(--base-size-8); - & .FormControl-input-trailingText { + & .FormControl-input-trailingVisualText { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; @@ -371,7 +371,7 @@ & .FormControl-input { padding-inline-end: calc(var(--control-medium-paddingInline-condensed) + var(--base-size-16) + var(--control-medium-gap)); } - &:has(.FormControl-input-trailingText) .FormControl-input { + &:has(.FormControl-input-trailingVisualText) .FormControl-input { padding-inline-end: 25% } } @@ -499,7 +499,7 @@ & .FormControl-input { padding-inline-end: calc(var(--control-large-paddingInline-normal) + var(--base-size-16) + var(--control-large-gap)); } - &:has(.FormControl-input-trailingText) .FormControl-input { + &:has(.FormControl-input-trailingVisualText) .FormControl-input { padding-inline-end: 25% } } diff --git a/app/lib/primer/forms/text_field.rb b/app/lib/primer/forms/text_field.rb index 9b11670d51..3391c400e7 100644 --- a/app/lib/primer/forms/text_field.rb +++ b/app/lib/primer/forms/text_field.rb @@ -46,16 +46,16 @@ def trailing_visual_render content = ActiveSupport::SafeBuffer.new # Use SafeBuffer for safe HTML concatenation # Render icon if specified - content << render(Primer::Beta::Octicon.new(icon: visual[:icon])) if visual[:icon] + 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()) { visual[:label] } if visual[:label] + 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])) if visual[:counter] + 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-trailingText") if visual[:text] + 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") diff --git a/previews/primer/alpha/text_field_preview.rb b/previews/primer/alpha/text_field_preview.rb index e280a619aa..a8390f7c65 100644 --- a/previews/primer/alpha/text_field_preview.rb +++ b/previews/primer/alpha/text_field_preview.rb @@ -181,7 +181,7 @@ def with_trailing_icon # @label With trailing text # @snapshot def with_trailing_text - render(Primer::Alpha::TextField.new( trailing_visual: { text: "Foobarbaz" }, name: "my-text-field", label: "My text field")) + render(Primer::Alpha::TextField.new( trailing_visual: { text: "minute" }, name: "my-text-field", label: "My text field")) end # @label With trailing counter diff --git a/test/components/alpha/text_field_test.rb b/test/components/alpha/text_field_test.rb index e906c1b3cd..e2ed7531e1 100644 --- a/test/components/alpha/text_field_test.rb +++ b/test/components/alpha/text_field_test.rb @@ -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 From 4fc03e8cfc790d8f4d7019cf3e6be2a614caf38a Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad <62008897+bsatarnejad@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:00:47 +0100 Subject: [PATCH 07/10] render the trailing elements in template Co-authored-by: Cameron Dutro --- app/lib/primer/forms/text_field.html.erb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/lib/primer/forms/text_field.html.erb b/app/lib/primer/forms/text_field.html.erb index 207ad1973e..509e200c33 100644 --- a/app/lib/primer/forms/text_field.html.erb +++ b/app/lib/primer/forms/text_field.html.erb @@ -17,8 +17,10 @@ <%= render(Primer::Beta::Octicon.new(icon: :"x-circle-fill")) %> <% end %> - <% if @input.trailing_visual %> - <%= trailing_visual_render %> + <% if (component = @input.trailing_visual_component) %> +
+ <%= render(component) %> +
<% end %> <% end %> <% end %> From 1bbaa39c5dd4c16be00c2c2104b2ad2f7f3f160c Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad <62008897+bsatarnejad@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:01:04 +0100 Subject: [PATCH 08/10] Update app/lib/primer/forms/text_field.rb Co-authored-by: Cameron Dutro --- app/lib/primer/forms/text_field.rb | 42 ++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/app/lib/primer/forms/text_field.rb b/app/lib/primer/forms/text_field.rb index 3391c400e7..19a97205f4 100644 --- a/app/lib/primer/forms/text_field.rb +++ b/app/lib/primer/forms/text_field.rb @@ -39,26 +39,40 @@ def auto_check_authenticity_token end end - def trailing_visual_render + def trailing_visual_component + return @trailing_visual_component if defined?(@trailing_visual_component) 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] + @trailing_visual_component = if (icon_arguments = visual[:icon]) + icon_arguments[:classes] = class_names( + icon_arguments.delete(:classes), + "FormControl-input-trailingVisualIcon" + ) - # Render counter if specified - content << render(Primer::Beta::Counter.new(count: visual[:counter], classes: "FormControl-input-trailingVisualCounter")) if visual[:counter] + 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" + ) - # Render text if specified - content << content_tag(:span, visual[:text], class: "FormControl-input-trailingVisualText") if visual[:text] + 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" + ) - # Wrap in the trailing visual container - content_tag(:span, content, class: "FormControl-input-trailingVisualWrap") + 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 From 472b01b1050aa9743a6a3f3bc90132f99dbbb703 Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Mon, 6 Jan 2025 14:39:13 +0100 Subject: [PATCH 09/10] fix preview issues and set a class for text so it can be truncated when it is too long --- app/components/primer/alpha/text_field.pcss | 7 ------- app/lib/primer/forms/dsl/text_field_input.rb | 13 +------------ app/lib/primer/forms/text_field.html.erb | 4 ++-- app/lib/primer/forms/text_field.rb | 8 +++++--- previews/primer/alpha/text_field_preview.rb | 14 ++++++++++---- 5 files changed, 18 insertions(+), 28 deletions(-) diff --git a/app/components/primer/alpha/text_field.pcss b/app/components/primer/alpha/text_field.pcss index 94707ab81b..8be1a57eb9 100644 --- a/app/components/primer/alpha/text_field.pcss +++ b/app/components/primer/alpha/text_field.pcss @@ -266,15 +266,8 @@ 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 */ diff --git a/app/lib/primer/forms/dsl/text_field_input.rb b/app/lib/primer/forms/dsl/text_field_input.rb index 9aa4dd23f2..8f22ca5920 100644 --- a/app/lib/primer/forms/dsl/text_field_input.rb +++ b/app/lib/primer/forms/dsl/text_field_input.rb @@ -18,7 +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)) + @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) @@ -55,17 +55,6 @@ 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 diff --git a/app/lib/primer/forms/text_field.html.erb b/app/lib/primer/forms/text_field.html.erb index 509e200c33..562f01098b 100644 --- a/app/lib/primer/forms/text_field.html.erb +++ b/app/lib/primer/forms/text_field.html.erb @@ -17,9 +17,9 @@ <%= render(Primer::Beta::Octicon.new(icon: :"x-circle-fill")) %> <% end %> - <% if (component = @input.trailing_visual_component) %> + <% if @input.trailing_visual %>
- <%= render(component) %> + <%= render(trailing_visual_component) %>
<% end %> <% end %> diff --git a/app/lib/primer/forms/text_field.rb b/app/lib/primer/forms/text_field.rb index 19a97205f4..b141e1a8dd 100644 --- a/app/lib/primer/forms/text_field.rb +++ b/app/lib/primer/forms/text_field.rb @@ -67,15 +67,17 @@ def trailing_visual_component "FormControl-input-trailingVisualCounter" ) - Primer::Beta::Counter.new(**counter_arguments)) + Primer::Beta::Counter.new(**counter_arguments) elsif (truncate_arguments = visual[:text]) # Render text if specified + truncate_arguments[:classes] = class_names( + truncate_arguments.delete(:classes), + "FormControl-input-trailingVisualText" + ) text = truncate_arguments.delete(:text) Primer::Beta::Truncate.new(**truncate_arguments).with_content(text) end end - - end end end diff --git a/previews/primer/alpha/text_field_preview.rb b/previews/primer/alpha/text_field_preview.rb index a8390f7c65..494d4d9f99 100644 --- a/previews/primer/alpha/text_field_preview.rb +++ b/previews/primer/alpha/text_field_preview.rb @@ -175,25 +175,31 @@ def monospace # @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")) + render(Primer::Alpha::TextField.new(trailing_visual: { icon: { 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")) + render(Primer::Alpha::TextField.new( trailing_visual: { text: { text: "minute" } }, name: "my-text-field", label: "My text field")) + end + + # @label With trailing long text + # @snapshot + def with_trailing_long_text + render(Primer::Alpha::TextField.new( trailing_visual: { text: { text: "Long trailing text" } }, 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")) + render(Primer::Alpha::TextField.new( trailing_visual: { counter: { 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")) + render(Primer::Alpha::TextField.new( trailing_visual: { label: { text: "Hello" } }, name: "my-text-field", label: "My text field")) end # @label With leading visual From 5ec5919622c3cfa30017f4783ab615f53b3d2beb Mon Sep 17 00:00:00 2001 From: Behrokh Satarnejad Date: Tue, 14 Jan 2025 12:05:20 +0100 Subject: [PATCH 10/10] set max-width for label and change the padding-inline-end of input to 25 percent --- app/components/primer/alpha/text_field.pcss | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/app/components/primer/alpha/text_field.pcss b/app/components/primer/alpha/text_field.pcss index 8be1a57eb9..c861012189 100644 --- a/app/components/primer/alpha/text_field.pcss +++ b/app/components/primer/alpha/text_field.pcss @@ -269,6 +269,14 @@ max-width: 25%; padding-left: var(--base-size-8); } + &:has( .FormControl-input-trailingVisualLabel) { + max-width: 25%; + padding-left: var(--base-size-8); + } + .FormControl-input-trailingVisualLabel { + overflow: hidden; + text-overflow: ellipsis; + } } /* TODO: replace with new Button component */ & .FormControl-input-trailingAction { @@ -367,6 +375,9 @@ &:has(.FormControl-input-trailingVisualText) .FormControl-input { padding-inline-end: 25% } + &:has(.FormControl-input-trailingVisualLabel) .FormControl-input { + padding-inline-end: 25% + } } /* @@ -495,6 +506,9 @@ &:has(.FormControl-input-trailingVisualText) .FormControl-input { padding-inline-end: 25% } + &:has(.FormControl-input-trailingVisualLabel) .FormControl-input { + padding-inline-end: 25% + } } &.FormControl-input-wrap--trailingText { @@ -502,7 +516,11 @@ padding-inline-end: 25%; } } - + &.FormControl-input-wrap--trailingLabel { + & .FormControl-input.FormControl-large { + padding-inline-end: 25%; + } + } /* ┌──────────────────┬──36px──┐ ╎ ┌──────────────┐ ┌────┐ ╎