-
Notifications
You must be signed in to change notification settings - Fork 129
Port Primer React Text Field styling, removing trailing text truncation #3801
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
base: main
Are you sure you want to change the base?
Port Primer React Text Field styling, removing trailing text truncation #3801
Conversation
|
a8cb584 to
0e473e4
Compare
There was a problem hiding this 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::TruncatetoPrimer::Beta::Textcomponent to remove default truncation - Refactored CSS from grid-based positioning to flexbox layout with new
FormControl-input-baseWrapwrapper - Updated clear button from plain
<button>element toPrimer::Beta::IconButtoncomponent 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 { |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
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.
| ::after { | |
| &::after { |
There was a problem hiding this comment.
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.
0e473e4 to
1ea1e4f
Compare
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
1ea1e4f to
af8e888
Compare
What are you trying to accomplish?
Screenshots
Integration
List the issues that this change affects.
Risk Assessment
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
inputelement. Primer React takes a completely different approach - hiding theinputnative 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:
.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-inputinput.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
Merge checklist
inputis wrapped in aauto-check.rounded-left-0override).OpenProject ticket (for internal tracking)
https://community.openproject.org/wp/69504