Skip to content

Conversation

@myabc
Copy link
Contributor

@myabc myabc commented Dec 11, 2025

What are you trying to accomplish?

  • Remove the default truncation from Text Field trailing visual text.
  • Brings Text Field trailing visual behavior in line with Primer React implementation.
  • Harmonize padding with Primer React implementation.

Screenshots

react_inputs_annotated_all localhost_4000_lookbook_preview_primer_alpha_text_field_options__display=%257B%2522theme%2522%253A%2522light%2522%257D(iPhone SE) (1)

Integration

List the issues that this change affects.

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?

This is a surprisingly hard to solve with the constraints of the current PVC implementation. PVC uses absolute positioning to layout leading/trailing visuals and actions above the actual input element. Primer React takes a completely different approach - hiding the input native styling and using a flexbox wrapper.

While it could be solved with JavaScript hacks (or possibly experimental selectors?), I actually think the simplest - albeit still complex - solution is to migrate to the Primer React approach.

Anything you want to highlight for special attention from reviewers?

Rough mapping:

Primer View Components Primer React
.FormControl-input-baseWrap (new class) .TextInputBaseWrapper
.FormControl-input-wrap .TextInputWrapper
.FormControl-input-trailingVisualWrap .TextInput-icon
.FormControl-input-leadingVisualWrap .TextInput-icon
.FormControl-input-trailingAction .TextInput-icon
.FormControl-input input
.FormControl-input-wrap--leadingVisual [data-leading-visual]
.FormControl-input-wrap--trailingVisual [data-trailing-visual]:not([data-trailing-action])
.FormControl-input-wrap--trailingAction [data-trailing-action]
.FormControl-input-wrap--small [data-size='small']
.FormControl-input-wrap--large [data-size='large']

Accessibility

  • No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

  • Fix known issues in implementation:
    • Fix padding when trailing visual/action present.
    • Fix padding when input is wrapped in a auto-check.
    • Fix implementation for lookbook input group examples (rounded-left-0 override).
  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

OpenProject ticket (for internal tracking)

https://community.openproject.org/wp/69504

@changeset-bot
Copy link

changeset-bot bot commented Dec 11, 2025

⚠️ No Changeset found

Latest commit: af8e888

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@myabc myabc force-pushed the bug/69504-text-field-trailing-text-truncation branch from a8cb584 to 0e473e4 Compare December 11, 2025 20:05
@myabc myabc marked this pull request as ready for review December 11, 2025 20:42
Copilot AI review requested due to automatic review settings December 11, 2025 20:42
@myabc myabc requested review from a team as code owners December 11, 2025 20:42
@myabc myabc requested a review from jonrohan December 11, 2025 20:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the default text truncation from Text Field trailing visual text to align with Primer React's implementation. The change replaces Primer::Beta::Truncate with Primer::Beta::Text for trailing visual text, while also introducing a significant CSS refactor that moves from a grid-based layout to a flexbox-based architecture for text field inputs.

Key Changes:

  • Changed trailing visual text from Primer::Beta::Truncate to Primer::Beta::Text component to remove default truncation
  • Refactored CSS from grid-based positioning to flexbox layout with new FormControl-input-baseWrap wrapper
  • Updated clear button from plain <button> element to Primer::Beta::IconButton component wrapped in a <span>

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
app/lib/primer/forms/text_field.rb Renamed variables from truncate_arguments to text_arguments, changed component from Primer::Beta::Truncate to Primer::Beta::Text, added FormControl-input-baseWrap class to field wrapper
app/lib/primer/forms/text_field.html.erb Replaced plain button clear button with Primer::Beta::IconButton component wrapped in a span with FormControl-input-trailingAction class
app/components/primer/alpha/text_field.pcss Major refactor introducing flexbox-based layout architecture with FormControl-input-baseWrap, new input styling approach, updated visual positioning, and new trailing action button styles

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

var(--borderWidth-thin)
); /* 29px */
@media (pointer: coarse) {
::after {
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The ::after pseudo-element selector is missing the & prefix. This means it will target any element's ::after instead of the button's ::after pseudo-element. This should be &::after to properly scope it to .FormControl-input-trailingActionButton.

Suggested change
::after {
&::after {

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was copied verbatim from packages/react/src/internal/components/TextInputInnerAction.module.css. As such, I think it's probably correct - the ::after should target the svg Element rather than the button.

@myabc myabc force-pushed the bug/69504-text-field-trailing-text-truncation branch from 0e473e4 to 1ea1e4f Compare December 11, 2025 21:20
Also renders clear button using an `IconButton`.

Rough mapping:

| Primer View Components                    | Primer React                                         |
| ----------------------------------------- | ---------------------------------------------------- |
| `.FormControl-input-baseWrap` (new class) | `.TextInputBaseWrapper`                              |
| `.FormControl-input-wrap`                 | `.TextInputWrapper`                                  |
| `.FormControl-input-trailingVisualWrap`   | `.TextInput-icon`                                    |
| `.FormControl-input-leadingVisualWrap`    | `.TextInput-icon`                                    |
| `.FormControl-input-trailingAction`       | `.TextInput-icon`                                    |
| `.FormControl-input`                      | `input`                                              |
| `.FormControl-input-wrap--leadingVisual`  | `[data-leading-visual]`                              |
| `.FormControl-input-wrap--trailingVisual` | `[data-trailing-visual]:not([data-trailing-action])` |
| `.FormControl-input-wrap--trailingAction` | `[data-trailing-action]`                             |
| `.FormControl-input-wrap--small`          | `[data-size='small']`                                |
| `.FormControl-input-wrap--large`          | `[data-size='large']`                                |

Closes primer#3800
Closes primer#3802
Closes primer#3803
@myabc myabc force-pushed the bug/69504-text-field-trailing-text-truncation branch from 1ea1e4f to af8e888 Compare December 11, 2025 21:23
@myabc myabc changed the title Remove Text Field trailing visual text truncation Port Primer React Text Field styling, removing trailing text truncation Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant