feat: add smooth glow/border animation on Input focus#1747
Open
pixeltannu wants to merge 4 commits into
Open
Conversation
github-actions
Bot
added
type:feature
📝 WalkthroughWalkthroughFive input widget classes ( ChangesInput Focus Border Style
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@packages/ui/src/EmailInput.ts`:
- Around line 143-148: The focus/blur logic in EmailInput is incorrectly wiping
the custom border color when focus is lost. Instead of setting
this._style.borderFg to undefined when this.isFocused is false, you need to
restore it to the original/configured border color value. Store the default or
caller-provided border color as a property during initialization (such as in a
constructor or property definition), then restore to that stored value in the
else block instead of setting it to undefined, so the component returns to its
properly configured style after blur.
In `@packages/ui/src/PasswordInput.ts`:
- Around line 128-133: In the PasswordInput class, the blur path unconditionally
sets this._style.borderFg to undefined, which discards any previously configured
borderFg value. Store the original borderFg value in a private field before
applying the focus effect (when this.isFocused is true), and restore that stored
value in the else branch instead of setting it to undefined.
In `@packages/ui/src/PathInput.ts`:
- Around line 212-217: The focus glow logic in PathInput is unsafely clearing
the borderFg style when unfocused by setting it to undefined, which removes any
custom theme values instead of preserving them. Modify the blur handling (the
else branch where isFocused is false) to restore the original borderFg color
value that existed before the focus glow was applied, rather than setting it to
undefined. Store and track the initial borderFg value so it can be properly
restored on blur, ensuring custom theme styles are preserved.
In `@packages/ui/src/SearchInput.ts`:
- Around line 105-110: The blur handling in the SearchInput component's
focus/blur logic is incorrectly resetting the border color to undefined instead
of reverting to the original theme color. Store the original border color value
from this._style.borderFg before applying the cyan glow effect in the isFocused
branch, then in the else branch (when not focused), restore this._style.borderFg
to the stored original value instead of setting it to undefined.
In `@packages/ui/src/TextArea.ts`:
- Around line 161-166: The TextArea component's focus glow effect is overriding
base styling by setting borderFg to undefined when the component loses focus.
Instead of resetting to undefined in the else block (when this.isFocused is
false), store the original border color value before applying the cyan glow
effect, and restore that original value when focus is lost. This ensures that
any custom borderFg styling is preserved and properly restored after the focus
state changes.
In `@packages/widgets/src/input/Button.ts`:
- Around line 145-147: In the Button class's _renderSelf method, replace the
direct usage of this._rect with this._getContentRect() to ensure the widget
respects style padding and insets consistently with all other input widgets in
the same directory. Change the line that destructures this._rect directly to
first call this._getContentRect() and store the result in a variable, then
destructure x, y, width, and height from that returned rect object.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ca7d5f8-a2ab-403b-81b1-e7d75146c621
📒 Files selected for processing (8)
packages/core/src/app/App.tspackages/core/src/events/types.tspackages/ui/src/EmailInput.tspackages/ui/src/PasswordInput.tspackages/ui/src/PathInput.tspackages/ui/src/SearchInput.tspackages/ui/src/TextArea.tspackages/widgets/src/input/Button.ts
Comment on lines
+143
to
+148
| // Apply glow effect on focus by changing border style | ||
| if (this.isFocused) { | ||
| this._style.borderFg = 'cyan'; | ||
| } else { | ||
| this._style.borderFg = undefined; | ||
| } |
There was a problem hiding this comment.
Focused glow currently wipes custom border color after blur
When focus is lost, setting this._style.borderFg = undefined permanently drops any caller-provided border color, so the component does not return to its configured style.
Proposed fix
protected _renderSelf(screen: Screen): void {
- // Apply glow effect on focus by changing border style
- if (this.isFocused) {
- this._style.borderFg = 'cyan';
- } else {
- this._style.borderFg = undefined;
- }
+ const baseBorderFg = this.style.borderFg;
+ this._style.borderFg = this.isFocused ? 'cyan' : baseBorderFg;
const rect = this._getContentRect();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Apply glow effect on focus by changing border style | |
| if (this.isFocused) { | |
| this._style.borderFg = 'cyan'; | |
| } else { | |
| this._style.borderFg = undefined; | |
| } | |
| protected _renderSelf(screen: Screen): void { | |
| const baseBorderFg = this.style.borderFg; | |
| this._style.borderFg = this.isFocused ? 'cyan' : baseBorderFg; | |
| const rect = this._getContentRect(); | |
| // ... rest of method | |
| } |
🤖 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 `@packages/ui/src/EmailInput.ts` around lines 143 - 148, The focus/blur logic
in EmailInput is incorrectly wiping the custom border color when focus is lost.
Instead of setting this._style.borderFg to undefined when this.isFocused is
false, you need to restore it to the original/configured border color value.
Store the default or caller-provided border color as a property during
initialization (such as in a constructor or property definition), then restore
to that stored value in the else block instead of setting it to undefined, so
the component returns to its properly configured style after blur.
Comment on lines
+128
to
+133
| // Apply glow effect on focus by changing border style | ||
| if (this.isFocused) { | ||
| this._style.borderFg = 'cyan'; | ||
| } else { | ||
| this._style.borderFg = undefined; | ||
| } |
There was a problem hiding this comment.
Blur path discards configured borderFg
The unfocused branch resets borderFg to undefined, which overrides prior style configuration instead of restoring it.
Proposed fix
protected _renderSelf(screen: Screen): void {
- // Apply glow effect on focus by changing border style
- if (this.isFocused) {
- this._style.borderFg = 'cyan';
- } else {
- this._style.borderFg = undefined;
- }
+ const baseBorderFg = this.style.borderFg;
+ this._style.borderFg = this.isFocused ? 'cyan' : baseBorderFg;
const rect = this._getContentRect();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Apply glow effect on focus by changing border style | |
| if (this.isFocused) { | |
| this._style.borderFg = 'cyan'; | |
| } else { | |
| this._style.borderFg = undefined; | |
| } | |
| const baseBorderFg = this.style.borderFg; | |
| this._style.borderFg = this.isFocused ? 'cyan' : baseBorderFg; | |
| const rect = this._getContentRect(); |
🤖 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 `@packages/ui/src/PasswordInput.ts` around lines 128 - 133, In the
PasswordInput class, the blur path unconditionally sets this._style.borderFg to
undefined, which discards any previously configured borderFg value. Store the
original borderFg value in a private field before applying the focus effect
(when this.isFocused is true), and restore that stored value in the else branch
instead of setting it to undefined.
Comment on lines
+212
to
+217
| // Apply glow effect on focus by changing border style | ||
| if (this.isFocused) { | ||
| this._style.borderFg = 'cyan'; | ||
| } else { | ||
| this._style.borderFg = undefined; | ||
| } |
There was a problem hiding this comment.
Focus glow logic is not style-safe
Clearing this._style.borderFg on blur removes custom theme values and leaves the widget with no configured border color.
Proposed fix
protected _renderSelf(screen: Screen): void {
- // Apply glow effect on focus by changing border style
- if (this.isFocused) {
- this._style.borderFg = 'cyan';
- } else {
- this._style.borderFg = undefined;
- }
+ const baseBorderFg = this.style.borderFg;
+ this._style.borderFg = this.isFocused ? 'cyan' : baseBorderFg;
const rect = this._getContentRect();🤖 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 `@packages/ui/src/PathInput.ts` around lines 212 - 217, The focus glow logic in
PathInput is unsafely clearing the borderFg style when unfocused by setting it
to undefined, which removes any custom theme values instead of preserving them.
Modify the blur handling (the else branch where isFocused is false) to restore
the original borderFg color value that existed before the focus glow was
applied, rather than setting it to undefined. Store and track the initial
borderFg value so it can be properly restored on blur, ensuring custom theme
styles are preserved.
Comment on lines
+161
to
+166
| // Apply glow effect on focus by changing border style | ||
| if (this.isFocused) { | ||
| this._style.borderFg = 'cyan'; | ||
| } else { | ||
| this._style.borderFg = undefined; | ||
| } |
There was a problem hiding this comment.
Border glow implementation overrides base styling
The blur path resets border color to undefined, so a custom borderFg is not restored after focus leaves the component.
Proposed fix
protected _renderSelf(screen: Screen): void {
- // Apply glow effect on focus by changing border style
- if (this.isFocused) {
- this._style.borderFg = 'cyan';
- } else {
- this._style.borderFg = undefined;
- }
+ const baseBorderFg = this.style.borderFg;
+ this._style.borderFg = this.isFocused ? 'cyan' : baseBorderFg;
const rect = this._getContentRect();🤖 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 `@packages/ui/src/TextArea.ts` around lines 161 - 166, The TextArea component's
focus glow effect is overriding base styling by setting borderFg to undefined
when the component loses focus. Instead of resetting to undefined in the else
block (when this.isFocused is false), store the original border color value
before applying the cyan glow effect, and restore that original value when focus
is lost. This ensures that any custom borderFg styling is preserved and properly
restored after the focus state changes.
Comment on lines
145
to
147
| protected _renderSelf(screen: Screen): void { | ||
| const { x, y, width, height } = this._rect; | ||
| if (width <= 0 || height <= 0) return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other widgets in the same directory handle _rect vs _getContentRect()
echo "=== Usage of _rect in _renderSelf across input widgets ==="
rg -n '_renderSelf|this\._rect|this\._getContentRect' packages/widgets/src/input/ --type ts -A2 -B2
echo ""
echo "=== _getContentRect implementation in base Widget ==="
ast-grep --pattern 'class Widget {
$$$
_getContentRect() {
$$$
}
$$$
}'Repository: Karanjot786/TermUI
Length of output: 6794
🏁 Script executed:
#!/bin/bash
# Read the full Button._renderSelf implementation
echo "=== Button._renderSelf full implementation (lines 145-205) ==="
sed -n '145,205p' packages/widgets/src/input/Button.ts
echo ""
echo "=== Button class definition and extends check ==="
head -50 packages/widgets/src/input/Button.ts | grep -E 'class Button|extends'
echo ""
echo "=== Compare with TextInput._renderSelf (similar control) ==="
sed -n '171,210p' packages/widgets/src/input/TextInput.tsRepository: Karanjot786/TermUI
Length of output: 4290
Use this._getContentRect() instead of this._rect in _renderSelf.
The coding guidelines require all widgets in packages/widgets/src/ to use this._getContentRect() in _renderSelf. Button currently uses this._rect directly (line 146), deviating from this requirement and from all other input widgets in the same directory (VirtualList, Slider, RangeInput, PinInput, List, Knob, CommandPalette, TextInput all use _getContentRect()).
Using _getContentRect() ensures the widget respects style padding and insets, even though Button manually renders borders. Change line 146 to:
const rect = this._getContentRect();
const { x, y, width, height } = rect;
🤖 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 `@packages/widgets/src/input/Button.ts` around lines 145 - 147, In the Button
class's _renderSelf method, replace the direct usage of this._rect with
this._getContentRect() to ensure the widget respects style padding and insets
consistently with all other input widgets in the same directory. Change the line
that destructures this._rect directly to first call this._getContentRect() and
store the result in a variable, then destructure x, y, width, and height from
that returned rect object.
Source: Coding guidelines
pixeltannu
force-pushed
the
feature/input-focus-animation
branch
from
6328b5f to
f956c1b
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (5)
packages/ui/src/TextArea.ts (1)
161-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFocused-state border mutation has no visible effect in
TextArea.Line 163 reapplies
'single', which is already the default border for this class, so focused and unfocused rendering remain the same.Suggested minimal fix
- if (this.isFocused) { - this._style.border = 'single'; - } else { - - } + const baseBorderFg = this.style.borderFg; + this._style.borderFg = this.isFocused + ? { type: 'named', name: 'cyan' } + : baseBorderFg;🤖 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 `@packages/ui/src/TextArea.ts` around lines 161 - 166, The border style being applied when isFocused is true in the TextArea class is set to 'single', which is already the default border style, making the focus state visually indistinguishable from the unfocused state. To create a visible glow effect on focus, change the border style in the if block where isFocused is checked to a different style value (such as 'double' or 'bold') that will provide a clear visual distinction when the text area is focused.packages/ui/src/SearchInput.ts (1)
110-115:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBorder style is not restored on blur in
SearchInput.After Line 112 sets
_style.border = 'single', the emptyelsebranch leaves that mutation in place. Once focused, the border stays enabled even when unfocused.Suggested minimal fix
- if (this.isFocused) { - this._style.border = 'single'; - } else { - - } + const baseBorder = this.style.border; + this._style.border = this.isFocused ? 'single' : baseBorder;🤖 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 `@packages/ui/src/SearchInput.ts` around lines 110 - 115, In the SearchInput class, the conditional block that handles the focus state is incomplete. When this.isFocused is true, the border is set to 'single' for the glow effect, but the else branch is empty and does not restore the border to its default state when the input loses focus. Add code in the else branch to reset this._style.border to its original value (likely an empty string or undefined) so that the glow effect is properly removed when the input is unfocused.packages/ui/src/EmailInput.ts (1)
143-148:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFocused border styling is currently a no-op in this widget.
On Line 145,
this._style.border = 'single'doesn’t create a focus-state change because the constructor already sets border to'single'by default. This means the new focus behavior is not actually visible forEmailInput.Suggested minimal fix
- if (this.isFocused) { - this._style.border = 'single'; - } else { - - } + const baseBorderFg = this.style.borderFg; + this._style.borderFg = this.isFocused + ? { type: 'named', name: 'cyan' } + : baseBorderFg;🤖 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 `@packages/ui/src/EmailInput.ts` around lines 143 - 148, The focused state border styling in the EmailInput class is ineffective because the constructor already sets the border to 'single' by default, so setting it to 'single' again when isFocused is true creates no visible change. Replace the border style assignment when isFocused is true with a different border style value (such as 'double' or 'bold') that creates a visible glow or focus effect, and in the else block explicitly set the border back to 'single' to represent the unfocused state.packages/ui/src/PasswordInput.ts (1)
270-275:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis focus-style block does not change visuals for
PasswordInput.Line 272 reapplies
'single'even though this component already has a single border from construction, so focused and unfocused states remain visually identical.Suggested minimal fix
- if (this.isFocused) { - this._style.border = 'single'; - } else { - - } + const baseBorderFg = this.style.borderFg; + this._style.borderFg = this.isFocused + ? { type: 'named', name: 'cyan' } + : baseBorderFg;🤖 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 `@packages/ui/src/PasswordInput.ts` around lines 270 - 275, The focus-style block in the PasswordInput class sets the border to 'single' when isFocused is true, but since PasswordInput already has a single border by default from construction, this produces no visual distinction between focused and unfocused states. To fix this, change the border style assignment when isFocused is true to use a visually distinct border style instead of 'single' (for example, 'double' or another style that provides clear visual feedback). Additionally, complete the else block to explicitly restore the original 'single' border style for the unfocused state to ensure the visual feedback is clear and consistent.packages/ui/src/PathInput.ts (1)
212-217:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFocus border change is ineffective for
PathInput.Line 214 sets
borderto'single', but this widget already defaults to'single', so the new focus block doesn’t produce a visible focused state.Suggested minimal fix
- if (this.isFocused) { - this._style.border = 'single'; - } else { - - } + const baseBorderFg = this.style.borderFg; + this._style.borderFg = this.isFocused + ? { type: 'named', name: 'cyan' } + : baseBorderFg;🤖 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 `@packages/ui/src/PathInput.ts` around lines 212 - 217, The focus state handler in PathInput sets the border to 'single' when isFocused is true, but since the widget already defaults to 'single' border style, this produces no visible change. In the if (this.isFocused) block, apply a different border style such as 'double' or 'bold', or add additional visual styling like a border color change to the this._style object to make the focused state visually distinct from the unfocused state.
🤖 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.
Duplicate comments:
In `@packages/ui/src/EmailInput.ts`:
- Around line 143-148: The focused state border styling in the EmailInput class
is ineffective because the constructor already sets the border to 'single' by
default, so setting it to 'single' again when isFocused is true creates no
visible change. Replace the border style assignment when isFocused is true with
a different border style value (such as 'double' or 'bold') that creates a
visible glow or focus effect, and in the else block explicitly set the border
back to 'single' to represent the unfocused state.
In `@packages/ui/src/PasswordInput.ts`:
- Around line 270-275: The focus-style block in the PasswordInput class sets the
border to 'single' when isFocused is true, but since PasswordInput already has a
single border by default from construction, this produces no visual distinction
between focused and unfocused states. To fix this, change the border style
assignment when isFocused is true to use a visually distinct border style
instead of 'single' (for example, 'double' or another style that provides clear
visual feedback). Additionally, complete the else block to explicitly restore
the original 'single' border style for the unfocused state to ensure the visual
feedback is clear and consistent.
In `@packages/ui/src/PathInput.ts`:
- Around line 212-217: The focus state handler in PathInput sets the border to
'single' when isFocused is true, but since the widget already defaults to
'single' border style, this produces no visible change. In the if
(this.isFocused) block, apply a different border style such as 'double' or
'bold', or add additional visual styling like a border color change to the
this._style object to make the focused state visually distinct from the
unfocused state.
In `@packages/ui/src/SearchInput.ts`:
- Around line 110-115: In the SearchInput class, the conditional block that
handles the focus state is incomplete. When this.isFocused is true, the border
is set to 'single' for the glow effect, but the else branch is empty and does
not restore the border to its default state when the input loses focus. Add code
in the else branch to reset this._style.border to its original value (likely an
empty string or undefined) so that the glow effect is properly removed when the
input is unfocused.
In `@packages/ui/src/TextArea.ts`:
- Around line 161-166: The border style being applied when isFocused is true in
the TextArea class is set to 'single', which is already the default border
style, making the focus state visually indistinguishable from the unfocused
state. To create a visible glow effect on focus, change the border style in the
if block where isFocused is checked to a different style value (such as 'double'
or 'bold') that will provide a clear visual distinction when the text area is
focused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 766966f1-08cc-47f8-8a7d-92b81cb67407
📒 Files selected for processing (5)
packages/ui/src/EmailInput.tspackages/ui/src/PasswordInput.tspackages/ui/src/PathInput.tspackages/ui/src/SearchInput.tspackages/ui/src/TextArea.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add smooth glow/border animation on Input focus by changing border color to cyan when an Input field is focused and resetting it on unfocus. This improves visual feedback and makes the UI feel more polished.
Related Issue
Closes #1734
What kind of change does this PR introduce?
Files Changed
Summary by CodeRabbit