Skip to content

fix(form-control): add transition styles - #8211

Merged
mcoker merged 5 commits into
patternfly:mainfrom
tarunvashishth:fix/number-input-disabled-animation-sync
Jul 31, 2026
Merged

fix(form-control): add transition styles#8211
mcoker merged 5 commits into
patternfly:mainfrom
tarunvashishth:fix/number-input-disabled-animation-sync

Conversation

@tarunvashishth

@tarunvashishth tarunvashishth commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Add transition to form-control so the input background and border changes animate in sync with the increment/decrement buttons when toggling isDisabled.

This resolves the visual delay where the buttons and input updated at different times.

Screen.Recording.2026-03-07.at.7.18.45.PM.mov

Fixes #7429

Summary by CodeRabbit

  • Enhancements
    • Improved form control animations by introducing configurable transition timing, duration, delay, and transition properties.
    • Updated related pseudo-element animations so they follow the same transition settings for smoother visual behavior.
  • Bug Fixes
    • Refined the disabled form control state styling so it no longer overrides border style during transitions, ensuring consistent appearance while remaining non-interactive.

@coderabbitai

coderabbitai Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The form control stylesheet adds configurable transition properties, applies them to the control and its pseudo-elements, and changes disabled pseudo-elements to transparent borders while preserving the disabled cursor.

Changes

Form control transitions

Layer / File(s) Summary
Transition token wiring
src/patternfly/components/FormControl/form-control.scss
Defines transition delay, timing function, duration, and properties; applies them to the form control and pseudo-elements; and sets disabled pseudo-element borders to transparent.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: srambach, lboehling

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows conventional commit format and matches the transition-style changes in form-control.
Linked Issues check ✅ Passed The SCSS changes add synchronized transitions and keep disabled borders visible, matching #7429's animation-sync fix.
Out of Scope Changes check ✅ Passed The only changes are scoped to form-control.scss and directly support the disabled transition fix.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@patternfly-build

patternfly-build commented Mar 6, 2026

Copy link
Copy Markdown
Collaborator

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/patternfly/components/FormControl/form-control.scss`:
- Around line 21-24: The new custom property
--#{$form-control}--TransitionDuration is introduced but not used everywhere
(the duration is still hardcoded/bypassed in the other rule), so update the
rules that set transition-duration to instead reference
var(--#{$form-control}--TransitionDuration); locate occurrences of
--#{$form-control}--TransitionDuration and replace any direct/two-token duration
usages (the block around the other transition declaration) so all transition
declarations use var(--#{$form-control}--TransitionDuration) as the single
source of truth (also update the duplicate occurrences noted around the 168-171
block).
- Line 454: The icon's transition is inheriting from
.pf-v6-c-form-control__utilities (which has no transition) causing snap changes;
update the stylesheet by either adding "transition: inherit;" to the
.pf-v6-c-form-control__utilities rule or by giving .pf-v6-c-form-control__icon
explicit transition longhand properties (e.g.,
transition-property/transition-duration/transition-timing-function) so the
.pf-v6-c-form-control__icon animates consistently with the root
.pf-v6-c-form-control.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 97b24b9f-5e90-45e6-a94d-9b2919332057

📥 Commits

Reviewing files that changed from the base of the PR and between d8cc066 and db5b511.

📒 Files selected for processing (1)
  • src/patternfly/components/FormControl/form-control.scss

Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
@coderabbitai
coderabbitai Bot requested review from lboehling, mcoker and srambach March 7, 2026 12:28

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/patternfly/components/FormControl/form-control.scss (1)

309-315: ⚠️ Potential issue | 🟠 Major

Keep the disabled border animatable.

The new transition only helps if the pseudo-borders stay present. Lines 314-315 switch both of them to none, so the outline still disappears instantly when .pf-m-disabled toggles. That leaves the field border snapping while adjacent pf-m-control buttons fade. Use transparent border colors here instead of removing the border style.

♻️ Proposed fix
   &.pf-m-disabled {
     --#{$form-control}--BackgroundColor: var(--#{$form-control}--m-disabled--BackgroundColor);
     --#{$form-control}--Color: var(--#{$form-control}--m-disabled--Color);
     --#{$form-control}--m-placeholder--Color: var(--#{$form-control}--m-disabled--Color);
     --#{$form-control}__toggle-icon--Color: var(--#{$form-control}--m-disabled__toggle-icon--Color);
-    --#{$form-control}--before--BorderStyle: none;
-    --#{$form-control}--after--BorderStyle: none;
+    --#{$form-control}--before--BorderColor: transparent;
+    --#{$form-control}--after--BorderColor: var(--#{$form-control}--m-disabled--after--BorderColor);
 
     cursor: not-allowed;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/patternfly/components/FormControl/form-control.scss` around lines 309 -
315, The disabled state currently sets --#{$form-control}--before--BorderStyle
and --#{$form-control}--after--BorderStyle to none which removes the
pseudo-borders and prevents the border transition; instead keep the border style
(don’t set them to none) and set the corresponding border-color variables to the
disabled/transparent values so the outline can animate (update usage around
.pf-m-disabled and the CSS vars --#{$form-control}--before--BorderStyle,
--#{$form-control}--after--BorderStyle and the matching
--#{$form-control}--before--BorderColor / --#{$form-control}--after--BorderColor
or their --m-disabled counterparts).
♻️ Duplicate comments (1)
src/patternfly/components/FormControl/form-control.scss (1)

25-28: ⚠️ Potential issue | 🟡 Minor

The new child transition hooks are still effectively disabled.

__control, __icon, and __toggle-icon all default to none/0s, and I don't see any override in this file that binds them back to the wrapper transition. That means the shell will animate, but child pieces that still paint their own state — for example the select background on Line 234 and the disabled toggle-icon color on Line 313 — will still snap.

Also applies to: 141-164, 219-222, 469-491

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/patternfly/components/FormControl/form-control.scss` around lines 25 -
28, The child transition custom properties for the form control components
(e.g., --#{$form-control}__control--TransitionDuration,
--#{$form-control}__control--TransitionProperty, and their __icon and
__toggle-icon counterparts) are set to 0s/none which disables child animations;
update these child variables to inherit or reference the wrapper transition
variables (the wrapper's transition timing/duration/property variables) so child
parts (select background, disabled toggle-icon color, etc.) follow the wrapper
animation, and remove hard-coded 0s/none defaults in the blocks that define
__control, __icon, and __toggle-icon (also apply same change to the other
occurrences covering the ranges noted) so children animate smoothly with the
wrapper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/patternfly/components/Button/button.scss`:
- Around line 942-945: The background shorthand currently always composes
disabled background with --#{$button}--BackgroundSurface which forces a tint
onto transparent variants like --#{$button}--m-plain--disabled--BackgroundColor:
transparent; change the rule that sets background (the line using
var(--#{$button}--disabled--BackgroundColor)
var(--#{$button}--BackgroundSurface)) so the surface is overridable—e.g. use the
surface custom property with a fallback of none or conditionally omit it—so
plain/transparent disabled buttons can set --#{$button}--BackgroundSurface: none
(or leave it unset) and stay transparent; update the background declaration
referencing --#{$button}--disabled--BackgroundColor and
--#{$button}--BackgroundSurface accordingly.

---

Outside diff comments:
In `@src/patternfly/components/FormControl/form-control.scss`:
- Around line 309-315: The disabled state currently sets
--#{$form-control}--before--BorderStyle and
--#{$form-control}--after--BorderStyle to none which removes the pseudo-borders
and prevents the border transition; instead keep the border style (don’t set
them to none) and set the corresponding border-color variables to the
disabled/transparent values so the outline can animate (update usage around
.pf-m-disabled and the CSS vars --#{$form-control}--before--BorderStyle,
--#{$form-control}--after--BorderStyle and the matching
--#{$form-control}--before--BorderColor / --#{$form-control}--after--BorderColor
or their --m-disabled counterparts).

---

Duplicate comments:
In `@src/patternfly/components/FormControl/form-control.scss`:
- Around line 25-28: The child transition custom properties for the form control
components (e.g., --#{$form-control}__control--TransitionDuration,
--#{$form-control}__control--TransitionProperty, and their __icon and
__toggle-icon counterparts) are set to 0s/none which disables child animations;
update these child variables to inherit or reference the wrapper transition
variables (the wrapper's transition timing/duration/property variables) so child
parts (select background, disabled toggle-icon color, etc.) follow the wrapper
animation, and remove hard-coded 0s/none defaults in the blocks that define
__control, __icon, and __toggle-icon (also apply same change to the other
occurrences covering the ranges noted) so children animate smoothly with the
wrapper.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: c3c35893-4680-434d-af6f-26dd7f33c25b

📥 Commits

Reviewing files that changed from the base of the PR and between db5b511 and a775f34.

📒 Files selected for processing (2)
  • src/patternfly/components/Button/button.scss
  • src/patternfly/components/FormControl/form-control.scss

Comment thread src/patternfly/components/Button/button.scss

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
src/patternfly/components/FormControl/form-control.scss (1)

25-28: ⚠️ Potential issue | 🟡 Minor

Propagate real transition values to the nested control/icon tokens.

These defaults still resolve the child longhands to 0s / none, so the consumers at Lines 220-223 and Lines 467-492 will snap while the wrapper animates. auto also is not a valid transition-timing-function, so these tokens will not behave as written once a non-zero transition is assigned.

♻️ Proposed fix
-  --#{$form-control}__control--TransitionDelay: 0s;
-  --#{$form-control}__control--TransitionTimingFunction: auto;
-  --#{$form-control}__control--TransitionDuration: 0s;
-  --#{$form-control}__control--TransitionProperty: none;
+  --#{$form-control}__control--TransitionDelay: var(--#{$form-control}--TransitionDelay);
+  --#{$form-control}__control--TransitionTimingFunction: var(--#{$form-control}--TransitionTimingFunction);
+  --#{$form-control}__control--TransitionDuration: var(--#{$form-control}--TransitionDuration);
+  --#{$form-control}__control--TransitionProperty: color, background-color;
...
-  --#{$form-control}__icon--TransitionDelay: 0s;
-  --#{$form-control}__icon--TransitionTimingFunction: auto;
-  --#{$form-control}__icon--TransitionDuration: 0s;
-  --#{$form-control}__icon--TransitionProperty: none;
+  --#{$form-control}__icon--TransitionDelay: var(--#{$form-control}--TransitionDelay);
+  --#{$form-control}__icon--TransitionTimingFunction: var(--#{$form-control}--TransitionTimingFunction);
+  --#{$form-control}__icon--TransitionDuration: var(--#{$form-control}--TransitionDuration);
+  --#{$form-control}__icon--TransitionProperty: color;
...
-  --#{$form-control}__toggle-icon--TransitionDelay: 0s;
-  --#{$form-control}__toggle-icon--TransitionTimingFunction: auto;
-  --#{$form-control}__toggle-icon--TransitionDuration: 0s;
-  --#{$form-control}__toggle-icon--TransitionProperty: none;
+  --#{$form-control}__toggle-icon--TransitionDelay: var(--#{$form-control}--TransitionDelay);
+  --#{$form-control}__toggle-icon--TransitionTimingFunction: var(--#{$form-control}--TransitionTimingFunction);
+  --#{$form-control}__toggle-icon--TransitionDuration: var(--#{$form-control}--TransitionDuration);
+  --#{$form-control}__toggle-icon--TransitionProperty: color;
Is `auto` a valid value for CSS `transition-timing-function`? Also, when a CSS custom property makes a `transition-*` longhand invalid at computed-value time, does the property fall back to its initial value?

Also applies to: 142-145, 162-165

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/patternfly/components/FormControl/form-control.scss` around lines 25 -
28, The nested control/icon transition token defaults are invalid/too-zeroed
(e.g., "--#{$form-control}__control--TransitionTimingFunction: auto" and 0s/none
for delay/duration/property) so child longhands snap; change the nested tokens
in form-control.scss to reference the wrapper's real transition tokens (e.g.,
replace "--#{$form-control}__control--TransitionDelay",
"--#{$form-control}__control--TransitionTimingFunction",
"--#{$form-control}__control--TransitionDuration",
"--#{$form-control}__control--TransitionProperty" with var(...) lookups of the
parent tokens like "--#{$form-control}--TransitionDelay",
"--#{$form-control}--TransitionTimingFunction" etc., and use a valid default
timing-function (e.g., ease) or a fallback in the var(...) so computed values
remain valid); apply the same change pattern to the other occurrences noted
(around lines 142-145 and 162-165) so child controls inherit/propagate the
wrapper's real transition values instead of 0s/none or the invalid "auto".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/patternfly/components/FormControl/form-control.scss`:
- Around line 25-28: The nested control/icon transition token defaults are
invalid/too-zeroed (e.g.,
"--#{$form-control}__control--TransitionTimingFunction: auto" and 0s/none for
delay/duration/property) so child longhands snap; change the nested tokens in
form-control.scss to reference the wrapper's real transition tokens (e.g.,
replace "--#{$form-control}__control--TransitionDelay",
"--#{$form-control}__control--TransitionTimingFunction",
"--#{$form-control}__control--TransitionDuration",
"--#{$form-control}__control--TransitionProperty" with var(...) lookups of the
parent tokens like "--#{$form-control}--TransitionDelay",
"--#{$form-control}--TransitionTimingFunction" etc., and use a valid default
timing-function (e.g., ease) or a fallback in the var(...) so computed values
remain valid); apply the same change pattern to the other occurrences noted
(around lines 142-145 and 162-165) so child controls inherit/propagate the
wrapper's real transition values instead of 0s/none or the invalid "auto".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: eea76722-b741-4da4-81b9-25bd1464f712

📥 Commits

Reviewing files that changed from the base of the PR and between a775f34 and 13c5b31.

📒 Files selected for processing (2)
  • src/patternfly/components/Button/button.scss
  • src/patternfly/components/FormControl/form-control.scss

@github-actions

Copy link
Copy Markdown

This PR has been automatically marked as stale because it has not had activity in the last 60 days.

@github-actions github-actions Bot added the Stale label May 17, 2026
Comment thread src/patternfly/components/Button/button.scss
Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
Comment thread src/patternfly/components/FormControl/form-control.scss
Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
Comment thread src/patternfly/components/FormControl/form-control.scss Outdated
@github-actions github-actions Bot removed the Stale label Jul 3, 2026
Address review feedback by handling the disabled animation sync entirely
from form-control.scss:

- Revert button.scss; the wrapper transition is sufficient.
- Drop the __control, __icon and __toggle-icon transition tokens and the
  places they were applied. Icons are out of scope here.
- Drop `transition: inherit` from the ::before/::after and __utilities
  rules.
- Restore `--before--BorderStyle: none` / `--after--BorderStyle: none` on
  the disabled modifier and remove the associated border color token.

The wrapper keeps the same transition properties and duration the buttons
use, so the input background and border stay in sync with the
increment/decrement buttons.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/patternfly/components/FormControl/form-control.scss (1)

168-171: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Apply the transition to the pseudo-elements that render the border.

The transition timing is only on .pf-v6-c-form-control; its ::before and ::after inherit the CSS variables but not the transition properties, so border-color changes on those pseudo-elements snap. Add the transition timing to &::before, &::after, and avoid animating the disabled-state border-style disappearance since changing border-style to none is discrete.

Proposed fix
   &::before,
   &::after {
     position: absolute;
     inset: 0;
     pointer-events: none;
     content: "";
+    transition-delay: var(--#{$form-control}--TransitionDelay);
+   ,
+    transition-timing-function: var(--#{$form-control}--TransitionTimingFunction);
+    transition-duration: var(--#{$form-control}--TransitionDuration);
+    transition-property: border-color;
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/patternfly/components/FormControl/form-control.scss` around lines 168 -
171, Update the transition declarations in the form-control styles so they also
apply to the border-rendering `&::before` and `&::after` pseudo-elements,
allowing their `border-color` changes to animate. Exclude `border-style` from
the transition to prevent animating the disabled-state switch to `none`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/patternfly/components/FormControl/form-control.scss`:
- Around line 168-171: Update the transition declarations in the form-control
styles so they also apply to the border-rendering `&::before` and `&::after`
pseudo-elements, allowing their `border-color` changes to animate. Exclude
`border-style` from the transition to prevent animating the disabled-state
switch to `none`.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 89ed4b34-b045-4f08-9ded-572b3a327354

📥 Commits

Reviewing files that changed from the base of the PR and between 13c5b31 and 723859d.

📒 Files selected for processing (1)
  • src/patternfly/components/FormControl/form-control.scss

@mcoker mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for making those updates! This looks great except for one thing. The borders (on ::before/::after) are not syncing with the button.

You can probably just do something like this

Inherit the wrapper transition on ::before/::after so their border-color
animates, and keep the borders present when disabled by making them
transparent instead of removing the border style.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/patternfly/components/FormControl/form-control.scss`:
- Around line 301-304: Update the disabled-state &::before and &::after rules in
the form-control styles to use the existing
--#{$form-control}--m-disabled--after--BorderColor token rather than hardcoding
transparent, preserving consumer overrides for both pseudo-elements; remove the
token only if it is intentionally no longer needed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 515ba278-9792-41ce-82fc-68c4b7e822b2

📥 Commits

Reviewing files that changed from the base of the PR and between 723859d and aa33a6b.

📒 Files selected for processing (1)
  • src/patternfly/components/FormControl/form-control.scss

Comment thread src/patternfly/components/FormControl/form-control.scss

@mcoker mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perfect, thank you! Visual regressions passed and number input looks good.

numberinputdisabled.mov

@mcoker
mcoker merged commit 6d74b9d into patternfly:main Jul 31, 2026
6 checks passed
@patternfly-build

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 6.6.0-prerelease.26 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug - NumberInput - isDisabled toggle animation out of sync

4 participants