Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 66 additions & 39 deletions packages/ui/src/Accordion.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
// ─────────────────────────────────────────────────────
// @termuijs/ui — Accordion widget
//
// A collapsible vertical section widget with keyboard focus,
// support for single/multi-open modes, and toggle event fires.
// ─────────────────────────────────────────────────────

// @termuijs/ui - Accordion widget
import { Widget } from '@termuijs/widgets';
import {
type Style,
Expand All @@ -22,9 +16,16 @@ export interface AccordionItem {
}

export interface AccordionOptions {
/** Allow multiple sections open at once. Default: false (one at a time) */
multi?: boolean;
onToggle?: (index: number, open: boolean) => void;
animationMs?: number;
}

interface AnimationState {
startTime: number;
opening: boolean;
visibleLines: number;
totalLines: number;
}

export class Accordion extends Widget {
Expand All @@ -33,6 +34,8 @@ export class Accordion extends Widget {
private _onToggle?: (index: number, open: boolean) => void;
private _focusIndex = 0;
private _openSet: Set<number> = new Set();
private _animationMs: number;
private _animations: Map<number, AnimationState> = new Map();

focusable = true;

Expand All @@ -41,20 +44,17 @@ export class Accordion extends Widget {
this._items = items;
this._multi = opts?.multi ?? false;
this._onToggle = opts?.onToggle;
this._animationMs = opts?.animationMs ?? 250;
}

setItems(items: AccordionItem[]): void {
this._items = items;
this._focusIndex = Math.min(this._focusIndex, Math.max(0, items.length - 1));
const keysToDelete: number[] = [];
for (const idx of this._openSet) {
if (idx >= items.length) {
keysToDelete.push(idx);
}
}
for (const key of keysToDelete) {
this._openSet.delete(key);
if (idx >= items.length) keysToDelete.push(idx);
}
for (const key of keysToDelete) this._openSet.delete(key);
this.markDirty();
Comment on lines 50 to 58

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear stale animation state when items are removed.

setItems prunes _openSet for invalid indices but leaves _animations untouched. If a section is animating when removed, its animation state becomes orphaned and may cause subtle rendering issues.

     setItems(items: AccordionItem[]): void {
         this._items = items;
         this._focusIndex = Math.min(this._focusIndex, Math.max(0, items.length - 1));
         const keysToDelete: number[] = [];
         for (const idx of this._openSet) {
             if (idx >= items.length) keysToDelete.push(idx);
         }
         for (const key of keysToDelete) this._openSet.delete(key);
+        for (const idx of this._animations.keys()) {
+            if (idx >= items.length) this._animations.delete(idx);
+        }
         this.markDirty();
     }
📝 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
setItems(items: AccordionItem[]): void {
this._items = items;
this._focusIndex = Math.min(this._focusIndex, Math.max(0, items.length - 1));
const keysToDelete: number[] = [];
for (const idx of this._openSet) {
if (idx >= items.length) {
keysToDelete.push(idx);
}
}
for (const key of keysToDelete) {
this._openSet.delete(key);
if (idx >= items.length) keysToDelete.push(idx);
}
for (const key of keysToDelete) this._openSet.delete(key);
this.markDirty();
setItems(items: AccordionItem[]): void {
this._items = items;
this._focusIndex = Math.min(this._focusIndex, Math.max(0, items.length - 1));
const keysToDelete: number[] = [];
for (const idx of this._openSet) {
if (idx >= items.length) keysToDelete.push(idx);
}
for (const key of keysToDelete) this._openSet.delete(key);
for (const idx of this._animations.keys()) {
if (idx >= items.length) this._animations.delete(idx);
}
this.markDirty();
}
🤖 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/Accordion.ts` around lines 50 - 58, In the setItems method,
after pruning invalid indices from the _openSet object, apply the same cleanup
logic to the _animations object. Create an additional loop that iterates through
_animations keys and collects any indices that are greater than or equal to the
new items length, then delete those stale animation state entries. This ensures
that when items are removed, both the open state and animation state are
synchronized and do not contain orphaned references to removed items.

}

Expand All @@ -64,12 +64,14 @@ export class Accordion extends Widget {

if (!this._multi) {
for (const openIdx of Array.from(this._openSet)) {
this._startAnimation(openIdx, false);
this._openSet.delete(openIdx);
this._onToggle?.(openIdx, false);
}
}

this._openSet.add(index);
this._startAnimation(index, true);
this._onToggle?.(index, true);
this.markDirty();
}
Expand All @@ -79,35 +81,55 @@ export class Accordion extends Widget {
if (!this._openSet.has(index)) return;

this._openSet.delete(index);
this._startAnimation(index, false);
this._onToggle?.(index, false);
this.markDirty();
}

private _startAnimation(index: number, opening: boolean): void {
const item = this._items[index];
if (!item) return;
const totalLines = item.body.split('\n').length;
this._animations.set(index, {
startTime: Date.now(),
opening,
visibleLines: opening ? 0 : totalLines,
totalLines,
});
}

private _getVisibleLines(index: number): number {
const anim = this._animations.get(index);
if (!anim) {
return this._openSet.has(index) ? this._items[index].body.split('\n').length : 0;
}
const elapsed = Date.now() - anim.startTime;
const progress = Math.min(1, elapsed / this._animationMs);
let visible: number;
if (anim.opening) {
visible = Math.floor(progress * anim.totalLines);
} else {
visible = Math.floor((1 - progress) * anim.totalLines);
}
if (progress >= 1) this._animations.delete(index);
return visible;
}
Comment on lines +101 to +116

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard against division by zero when animationMs is zero.

If a user sets animationMs: 0 to disable animation, elapsed / this._animationMs produces NaN (when elapsed is also 0) or Infinity, which propagates through the visibility calculation. Consider treating zero or negative animationMs as an instant transition:

     private _getVisibleLines(index: number): number {
         const anim = this._animations.get(index);
         if (!anim) {
             return this._openSet.has(index) ? this._items[index].body.split('\n').length : 0;
         }
+        if (this._animationMs <= 0) {
+            this._animations.delete(index);
+            return this._openSet.has(index) ? anim.totalLines : 0;
+        }
         const elapsed = Date.now() - anim.startTime;
         const progress = Math.min(1, elapsed / this._animationMs);
🤖 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/Accordion.ts` around lines 101 - 116, The _getVisibleLines
method does not guard against division by zero when animationMs is zero or
negative. Add a guard check after retrieving the animation object but before
calculating elapsed time and progress. If this._animationMs is less than or
equal to zero, immediately return the final state: return anim.totalLines if
anim.opening is true, otherwise return 0. This ensures instant transitions when
animation duration is disabled while avoiding NaN or Infinity values from the
division operation.


handleKey(event: KeyEvent): void {
if (this._items.length === 0) return;
const key = event.key?.toLowerCase();

switch (key) {
case 'up':
if (this._focusIndex > 0) {
this._focusIndex--;
this.markDirty();
}
if (this._focusIndex > 0) { this._focusIndex--; this.markDirty(); }
break;
case 'down':
if (this._focusIndex < this._items.length - 1) {
this._focusIndex++;
this.markDirty();
}
if (this._focusIndex < this._items.length - 1) { this._focusIndex++; this.markDirty(); }
break;
case 'enter':
case 'space': {
const isOpened = this._openSet.has(this._focusIndex);
if (isOpened) {
this.closeSection(this._focusIndex);
} else {
this.openSection(this._focusIndex);
}
if (isOpened) this.closeSection(this._focusIndex);
else this.openSection(this._focusIndex);
break;
}
}
Expand All @@ -120,38 +142,43 @@ export class Accordion extends Widget {

const attrs = styleToCellAttrs(this.style);
let currentY = y;
let anyAnimating = false;

for (let i = 0; i < this._items.length; i++) {
if (currentY >= y + height) break;

const item = this._items[i];
const isOpen = this._openSet.has(i);
const isFocused = i === this._focusIndex;
const anim = this._animations.get(i);
if (anim) anyAnimating = true;

const progress = anim
? Math.min(1, (Date.now() - anim.startTime) / this._animationMs)
: (isOpen ? 1 : 0);
const indicator = progress > 0.5 ? 'v ' : '> ';

// 1. Render Title Bar
const indicator = isOpen ? (caps.unicode ? '▼ ' : 'v ') : (caps.unicode ? '▶ ' : '> ');
const titleText = `${indicator}${item.title}`;
const titleText = indicator + item.title;
const titleAttrs = {
...attrs,
fg: isFocused ? ({ type: 'named' as const, name: 'cyan' as const }) : attrs.fg,
bold: isFocused
};

const paddedTitle = titleText.padEnd(width).slice(0, width);
screen.writeString(x, currentY, paddedTitle, titleAttrs);
screen.writeString(x, currentY, titleText.padEnd(width).slice(0, width), titleAttrs);
currentY++;

// 2. Render Body content if open
if (isOpen) {
const bodyLines = item.body.split('\n');
const visibleLines = this._getVisibleLines(i);
if (visibleLines > 0) {
const bodyLines = item.body.split('\n').slice(0, visibleLines);
for (const line of bodyLines) {
if (currentY >= y + height) break;

const indentedLine = ` ${line}`;
screen.writeString(x, currentY, indentedLine.padEnd(width).slice(0, width), attrs);
screen.writeString(x, currentY, (' ' + line).padEnd(width).slice(0, width), attrs);
currentY++;
}
}
}

if (anyAnimating) setTimeout(() => this.markDirty(), 16);
}
}
140 changes: 82 additions & 58 deletions packages/ui/src/Toast.ts
Original file line number Diff line number Diff line change
@@ -1,74 +1,98 @@
// Toast auto-dismiss notification
// Toast - auto-dismiss notification
import { Widget } from '@termuijs/widgets';
import { type Screen, mergeStyles, defaultStyle, styleToCellAttrs, caps } from '@termuijs/core';

export type ToastType = 'info' | 'success' | 'warning' | 'error';
export interface ToastMessage { text: string; type: ToastType; expireAt: number; }

export interface ToastMessage {
text: string;
type: ToastType;
expireAt: number;
createdAt: number;
}

export interface ToastOptions {
position?: 'top-right' | 'bottom-right' | 'top-left' | 'bottom-left';
durationMs?: number;
maxVisible?: number;
/** Enable screen reader announcements (default: true) */
announce?: boolean;
position?: 'top-right' | 'bottom-right' | 'top-left' | 'bottom-left';
durationMs?: number;
maxVisible?: number;
announce?: boolean;
animationMs?: number;
}

const ICONS_UNICODE: Record<ToastType, string> = { info: '', success: '', warning: '', error: '' };
const ICONS_UNICODE: Record<ToastType, string> = { info: 'i', success: '+', warning: '!', error: 'x' };
const ICONS_ASCII: Record<ToastType, string> = { info: 'i', success: '+', warning: '!', error: 'x' };
const COLORS: Record<ToastType, string> = { info: 'cyan', success: 'green', warning: 'yellow', error: 'red' };

export class Toast extends Widget {
private _messages: ToastMessage[] = [];
private _position: string;
private _durationMs: number;
private _maxVisible: number;
private _announce: boolean;
private _messages: ToastMessage[] = [];
private _position: string;
private _durationMs: number;
private _maxVisible: number;
private _announce: boolean;
private _animationMs: number;

constructor(options: ToastOptions = {}) {
super(mergeStyles(defaultStyle(), {}));
this._position = options.position ?? 'top-right';
this._durationMs = options.durationMs ?? 3000;
this._maxVisible = options.maxVisible ?? 5;
this._announce = options.announce ?? true;
}
constructor(options: ToastOptions = {}) {
super(mergeStyles(defaultStyle(), {}));
this._position = options.position ?? 'top-right';
this._durationMs = options.durationMs ?? 3000;
this._maxVisible = options.maxVisible ?? 5;
this._announce = options.announce ?? true;
this._animationMs = options.animationMs ?? 300;
}

push(text: string, type: ToastType = 'info'): void {
this._messages.push({ text, type, expireAt: Date.now() + this._durationMs });
this.markDirty();
if (this._announce) {
this._announceToScreenReader(text, type);
}
}
info(text: string): void { this.push(text, 'info'); }
success(text: string): void { this.push(text, 'success'); }
warning(text: string): void { this.push(text, 'warning'); }
error(text: string): void { this.push(text, 'error'); }
push(text: string, type: ToastType = 'info'): void {
const now = Date.now();
this._messages.push({ text, type, expireAt: now + this._durationMs, createdAt: now });
this.markDirty();
if (this._announce) this._announceToScreenReader(text, type);
}

private _announceToScreenReader(text: string, _type: ToastType): void {
try {
const announcement = `[${text}]`;
const oscSequence = `\x1b]777;notify;TermUI;${announcement}\x07`;
process.stderr.write(oscSequence);
} catch {
// Silently fail if stderr is not writable
}
}
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;
const visible = this._messages.slice(-this._maxVisible);
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;
const icons = caps.unicode ? ICONS_UNICODE : ICONS_ASCII;
const attrs = styleToCellAttrs(this.style);
for (let i = 0; i < visible.length; i++) {
const m = visible[i];
const label = ` ${icons[m.type]} ${m.text} `.slice(0, tw).padEnd(tw);
screen.writeString(sx, sy + i, label, { ...attrs, fg: { type: 'named', name: COLORS[m.type] as any }, bold: true }); // as any: COLORS values are string but Style.name expects NamedColor; types need aligning
}
info(text: string): void { this.push(text, 'info'); }
success(text: string): void { this.push(text, 'success'); }
warning(text: string): void { this.push(text, 'warning'); }
error(text: string): void { this.push(text, 'error'); }

private _announceToScreenReader(text: string, _type: ToastType): void {
try {
process.stderr.write('\x1b]777;notify;TermUI;[' + text + ']\x07');
} catch { }
Comment on lines +55 to +58

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Escape control characters before writing OSC notifications.

text is interpolated raw into an OSC sequence. If it contains ESC, BEL, or ST terminators, it can break the payload and inject terminal controls.

Suggested fix
   private _announceToScreenReader(text: string, _type: ToastType): void {
     try {
-      process.stderr.write('\x1b]777;notify;TermUI;[' + text + ']\x07');
+      const safe = text.replace(/[\x07\x1b\x9c]/g, ' ');
+      process.stderr.write(`\x1b]777;notify;TermUI;[${safe}]\x07`);
     } catch { }
   }
📝 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
private _announceToScreenReader(text: string, _type: ToastType): void {
try {
process.stderr.write('\x1b]777;notify;TermUI;[' + text + ']\x07');
} catch { }
private _announceToScreenReader(text: string, _type: ToastType): void {
try {
const safe = text.replace(/[\x07\x1b\x9c]/g, ' ');
process.stderr.write(`\x1b]777;notify;TermUI;[${safe}]\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, In the
_announceToScreenReader method, the text parameter is being interpolated
directly into the OSC sequence string without escaping control characters.
Control characters like ESC, BEL, and ST terminators in the text can break the
OSC payload and inject unwanted terminal controls. Escape these control
characters in the text parameter before concatenating it into the OSC
notification sequence that is written to process.stderr.

}

private _getAnimationProgress(createdAt: number, expireAt: number): number {
const now = Date.now();
const elapsed = now - createdAt;
const remaining = expireAt - now;
if (remaining < this._animationMs) return Math.max(0, remaining / this._animationMs);
if (elapsed < this._animationMs) return elapsed / this._animationMs;
return 1;
}

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;
const visible = this._messages.slice(-this._maxVisible);
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;
Comment on lines +76 to +80

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard tiny content widths before truncation math.

When width <= 2, tw becomes non-positive, which makes slice(0, tw)/progress * tw behave incorrectly and causes unstable rendering output.

Suggested fix
     const { x, y, width, height } = this._rect;
+    if (width <= 2 || height <= 2) return;
     const visible = this._messages.slice(-this._maxVisible);
     const tw = Math.min(40, width - 2);

Also applies to: 86-88

🤖 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 truncation width
calculation using Math.min(40, width - 2) can result in a non-positive value for
tw when the content width is 2 or less, causing incorrect behavior in subsequent
slice and multiplication operations. Add a guard check before the tw calculation
in the positioning block to validate that width is greater than 2, and handle
the edge case appropriately by either returning early or ensuring tw maintains a
minimum safe value before it is used in any string slicing or mathematical
operations that depend on a positive value.

const icons = caps.unicode ? ICONS_UNICODE : ICONS_ASCII;
const attrs = styleToCellAttrs(this.style);
for (let i = 0; i < visible.length; i++) {
const m = visible[i];
const progress = this._getAnimationProgress(m.createdAt, m.expireAt);
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 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove unannotated as any cast in cell attrs.

This bypasses strict typing and violates the TS guideline for any/type assertions without inline justification.

Suggested fix
-import { type Screen, mergeStyles, defaultStyle, styleToCellAttrs, caps } from '`@termuijs/core`';
+import { type Screen, type Color, mergeStyles, defaultStyle, styleToCellAttrs, caps } from '`@termuijs/core`';
@@
-const COLORS: Record<ToastType, string> = { info: 'cyan', success: 'green', warning: 'yellow', error: 'red' };
+const COLORS: Record<ToastType, Color> = {
+  info: { type: 'named', name: 'cyan' },
+  success: { type: 'named', name: 'green' },
+  warning: { type: 'named', name: 'yellow' },
+  error: { type: 'named', name: '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: COLORS[m.type], bold: true });

As per coding guidelines, "**/*.{ts,tsx}: Use TypeScript strict mode. No any without an inline comment explaining why... No type assertions without an inline comment explaining why."

🤖 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 type assertion `as any` applied to
`COLORS[m.type]` in the cell attributes passed to the `screen.writeString()`
method lacks an inline comment explaining why it is necessary, which violates
the TypeScript strict mode guidelines. Either remove the `as any` cast entirely
if it is not needed, or add an inline comment immediately after the cast
explaining the reason for bypassing strict typing. The cast is located in the
`fg` property configuration within the attributes object on the line with
`screen.writeString(sx, sy + i, label, ...)`.

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);
}
}
Loading
Loading