feat: add slide-in/fade-out animation for Toast notifications#1740
feat: add slide-in/fade-out animation for Toast notifications#1740pixeltannu wants to merge 1 commit into
Conversation
github-actions
Bot
added
type:feature
📝 WalkthroughWalkthrough
ChangesToast Slide-in Animation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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: 4
🤖 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/Toast.ts`:
- Around line 55-58: The _announceToScreenReader method embeds the raw text
parameter directly into an OSC control sequence on line 57, which is a security
risk as control characters like ESC, BEL, or ST in the text could break the
sequence or inject malicious commands. Sanitize the text parameter by escaping
or removing control bytes and special characters before concatenating it into
the OSC notification sequence to ensure only safe characters are embedded in the
terminal control code.
- Around line 91-97: The timeout scheduling in the animation check loop creates
a new timer every render cycle when anyAnimating is true, causing timer stacking
and unnecessary markDirty() calls. Add a property to track the pending animation
timer ID (for example, _pendingAnimationTimerId), and only schedule a new
setTimeout call when no timer is already pending. When the timer fires and calls
markDirty(), reset the tracking property to null or clear it so the next
animation cycle can schedule a fresh timer if needed.
- Around line 76-80: The calculation of tw on line 76 can produce zero or
negative values when width is less than 3, which causes incorrect positioning in
the sx calculation on line 79 and potential toast overlap issues. Add a guard
clause before the tw variable assignment to check if width is less than 3, and
if so, return early from the function since the widget geometry is unusable for
rendering. This ensures sx positioning calculations only proceed with valid,
non-negative text width values.
- Line 89: The COLORS object on line 89 is typed as Record<ToastType, string>
but should be explicitly typed as Record<ToastType, NamedColor> since all values
('cyan', 'green', 'yellow', 'red') are valid NamedColor values. Change the type
definition of the COLORS map from Record<ToastType, string> to Record<ToastType,
NamedColor>, then remove the unnecessary as any cast from the usage on line 89
where COLORS[m.type] is referenced. Ensure the NamedColor type is imported from
the appropriate module if not already present.
🪄 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: 963ce9c8-d29e-4aab-a16c-cdbda343ebfc
📒 Files selected for processing (1)
packages/ui/src/Toast.ts
| private _announceToScreenReader(text: string, _type: ToastType): void { | ||
| try { | ||
| process.stderr.write('\x1b]777;notify;TermUI;[' + text + ']\x07'); | ||
| } catch { } |
There was a problem hiding this comment.
Escape toast text before writing OSC notification sequence
Line 57 embeds raw text into an OSC control sequence. If text contains ESC/BEL/ST characters, it can terminate/inject terminal control codes. Sanitize control bytes before concatenation.
Suggested fix
private _announceToScreenReader(text: string, _type: ToastType): void {
try {
- process.stderr.write('\x1b]777;notify;TermUI;[' + text + ']\x07');
+ const safeText = text.replace(/[\x07\x1b\x9c]/g, '');
+ process.stderr.write('\x1b]777;notify;TermUI;[' + safeText + ']\x07');
} catch { }
}🤖 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/Toast.ts` around lines 55 - 58, The _announceToScreenReader
method embeds the raw text parameter directly into an OSC control sequence on
line 57, which is a security risk as control characters like ESC, BEL, or ST in
the text could break the sequence or inject malicious commands. Sanitize the
text parameter by escaping or removing control bytes and special characters
before concatenating it into the OSC notification sequence to ensure only safe
characters are embedded in the terminal control code.
| const tw = Math.min(40, width - 2); | ||
| const isRight = this._position.includes('right'); | ||
| const isBottom = this._position.includes('bottom'); | ||
| const sx = isRight ? x + width - tw - 1 : x + 1; | ||
| const sy = isBottom ? y + height - visible.length - 1 : y + 1; |
There was a problem hiding this comment.
Guard narrow widget widths before computing toast text width
Line 76 can produce tw <= 0 when width < 3, which then skews sx (Line 79) and can cause incorrect draw coordinates/overlap. Clamp width and early-return on unusable geometry.
Suggested fix
protected _renderSelf(screen: Screen): void {
const now = Date.now();
this._messages = this._messages.filter(m => m.expireAt > now);
if (this._messages.length === 0) return;
const { x, y, width, height } = this._rect;
+ if (width < 3 || height < 2) return;
const visible = this._messages.slice(-this._maxVisible);
- const tw = Math.min(40, width - 2);
+ const tw = Math.max(1, Math.min(40, width - 2));🤖 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/Toast.ts` around lines 76 - 80, The calculation of tw on line
76 can produce zero or negative values when width is less than 3, which causes
incorrect positioning in the sx calculation on line 79 and potential toast
overlap issues. Add a guard clause before the tw variable assignment to check if
width is less than 3, and if so, return early from the function since the widget
geometry is unusable for rendering. This ensures sx positioning calculations
only proceed with valid, non-negative text width values.
| const fullLabel = (' ' + icons[m.type] + ' ' + m.text + ' ').slice(0, tw).padEnd(tw); | ||
| const visibleChars = Math.floor(progress * tw); | ||
| const label = fullLabel.slice(0, visibleChars).padEnd(tw); | ||
| screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] as any }, bold: true }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify assertion/any usage in the changed file and strict-mode declaration in repo tsconfig(s)
rg -n --type=ts '\bas any\b|@\s*ts-ignore|as\s+[A-Za-z0-9_<>{}\[\]\|]+' packages/ui/src/Toast.ts
fd -i '^tsconfig(\..+)?\.json$' | xargs -r rg -n '"strict"\s*:\s*true'Repository: Karanjot786/TermUI
Length of output: 635
🏁 Script executed:
# Get context around line 89 in Toast.ts
sed -n '80,100p' packages/ui/src/Toast.ts
# Find where COLORS is defined
rg -n 'COLORS\s*[:=]' packages/ui/src/Toast.ts
# Check the entire file structure
wc -l packages/ui/src/Toast.tsRepository: Karanjot786/TermUI
Length of output: 1151
🏁 Script executed:
# Get the top of Toast.ts to see imports and type definitions
head -30 packages/ui/src/Toast.ts
# Find ToastType definition
rg -n 'type ToastType|enum ToastType' packages/ui/src/Toast.tsRepository: Karanjot786/TermUI
Length of output: 1174
🏁 Script executed:
# Look for Color type definition in the `@termuijs/core` package
find packages -name "*.ts" -o -name "*.tsx" | xargs rg -l "type Color|interface Color" | head -5
# Search for the Color type more broadly
rg -A 5 "type Color\s*=|interface Color\s*\{" packages/core/src/Repository: Karanjot786/TermUI
Length of output: 650
🏁 Script executed:
# Find NamedColor type definition
rg -A 5 "type NamedColor" packages/core/src/style/Color.tsRepository: Karanjot786/TermUI
Length of output: 311
Type COLORS as Record<ToastType, NamedColor> to eliminate the as any cast
Line 89 uses COLORS[m.type] as any unnecessarily. The COLORS map should be explicitly typed as Record<ToastType, NamedColor> instead of Record<ToastType, string>. All values ('cyan', 'green', 'yellow', 'red') are valid NamedColor values, so the cast can be removed entirely. This eliminates the type assertion that violates the no-any/no-assertions-without-comments guidelines.
Suggested change
-const COLORS: Record<ToastType, string> = { info: 'cyan', success: 'green', warning: 'yellow', error: 'red' };
+const COLORS: Record<ToastType, NamedColor> = { info: 'cyan', success: 'green', warning: 'yellow', error: 'red' };Then remove the as any cast on line 89:
- screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] as any }, bold: true });
+ screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] }, bold: true });Also add the import for NamedColor if not already present:
-import { type Screen, mergeStyles, defaultStyle, styleToCellAttrs, caps } from '`@termuijs/core`';
+import { type Screen, type NamedColor, mergeStyles, defaultStyle, styleToCellAttrs, caps } from '`@termuijs/core`';📝 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.
| screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] as any }, bold: true }); | |
| import { type Screen, type NamedColor, mergeStyles, defaultStyle, styleToCellAttrs, caps } from '`@termuijs/core`'; |
| screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] as any }, bold: true }); | |
| const COLORS: Record<ToastType, NamedColor> = { info: 'cyan', success: 'green', warning: 'yellow', error: 'red' }; |
| screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] as any }, bold: true }); | |
| screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] }, bold: true }); |
🤖 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/Toast.ts` at line 89, The COLORS object on line 89 is typed
as Record<ToastType, string> but should be explicitly typed as Record<ToastType,
NamedColor> since all values ('cyan', 'green', 'yellow', 'red') are valid
NamedColor values. Change the type definition of the COLORS map from
Record<ToastType, string> to Record<ToastType, NamedColor>, then remove the
unnecessary as any cast from the usage on line 89 where COLORS[m.type] is
referenced. Ensure the NamedColor type is imported from the appropriate module
if not already present.
Source: Coding guidelines
| const anyAnimating = visible.some(m => { | ||
| const elapsed = now - m.createdAt; | ||
| const remaining = m.expireAt - now; | ||
| return elapsed < this._animationMs || remaining < this._animationMs; | ||
| }); | ||
| if (anyAnimating) setTimeout(() => this.markDirty(), 16); | ||
| } |
There was a problem hiding this comment.
Avoid scheduling overlapping animation timers every render
Lines 91-97 schedule a new timeout whenever anyAnimating is true. Repeated renders can stack multiple pending timers and cause redundant markDirty() churn. Track one pending timer ID and only schedule when none exists.
Suggested fix
export class Toast extends Widget {
private _messages: ToastMessage[] = [];
+ private _animationTimer: ReturnType<typeof setTimeout> | null = null;
@@
- if (anyAnimating) setTimeout(() => this.markDirty(), 16);
+ if (anyAnimating && this._animationTimer === null) {
+ this._animationTimer = setTimeout(() => {
+ this._animationTimer = null;
+ this.markDirty();
+ }, 16);
+ }
}
}📝 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.
| const anyAnimating = visible.some(m => { | |
| const elapsed = now - m.createdAt; | |
| const remaining = m.expireAt - now; | |
| return elapsed < this._animationMs || remaining < this._animationMs; | |
| }); | |
| if (anyAnimating) setTimeout(() => this.markDirty(), 16); | |
| } | |
| const anyAnimating = visible.some(m => { | |
| const elapsed = now - m.createdAt; | |
| const remaining = m.expireAt - now; | |
| return elapsed < this._animationMs || remaining < this._animationMs; | |
| }); | |
| if (anyAnimating && this._animationTimer === null) { | |
| this._animationTimer = setTimeout(() => { | |
| this._animationTimer = null; | |
| this.markDirty(); | |
| }, 16); | |
| } | |
| } |
🤖 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/Toast.ts` around lines 91 - 97, The timeout scheduling in the
animation check loop creates a new timer every render cycle when anyAnimating is
true, causing timer stacking and unnecessary markDirty() calls. Add a property
to track the pending animation timer ID (for example, _pendingAnimationTimerId),
and only schedule a new setTimeout call when no timer is already pending. When
the timer fires and calls markDirty(), reset the tracking property to null or
clear it so the next animation cycle can schedule a fresh timer if needed.
|
@Karanjot786 please mearge my issue |
1 participant
Closes #1732
Changes
createdAttimestamp toToastMessageinterfaceanimationMsoption toToastOptions_getAnimationProgress()method for slide-in/fade-out effectsetTimeoutwith 16ms interval for ~60fps re-renderSummary by CodeRabbit
Release Notes
New Features
Improvements