fix(tui): cached loader layout across shimmer frames
Separated stable plain-text layout from animated ANSI coloring so 30fps shimmer updates no longer repeat wrapping and width measurement. Fixes #5230
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed animated Loader ANSI updates invalidating stable text layout, avoiding repeated wrapping and width measurement on shimmer-only frames ([#5230](https://github.com/can1357/oh-my-pi/issues/5230)).
|
||||
|
||||
## [16.4.5] - 2026-07-11
|
||||
|
||||
### Added
|
||||
|
||||
@@ -1,28 +1,29 @@
|
||||
import type { TUI } from "../tui";
|
||||
import { sliceByColumn, visibleWidth } from "../utils";
|
||||
import { getPaddingX, sliceByColumn, visibleWidth } from "../utils";
|
||||
import { Text } from "./text";
|
||||
|
||||
/**
|
||||
* Loader component. Spinner frames advance at `SPINNER_ADVANCE_MS`.
|
||||
*
|
||||
* Message colorizers that are time-dependent can opt into 30fps redraws by
|
||||
* setting `animated` to `true` on the function object.
|
||||
*/
|
||||
const RENDER_INTERVAL_MS = 1000 / 30;
|
||||
const SPINNER_ADVANCE_MS = 80;
|
||||
|
||||
type ColorFn = (str: string) => string;
|
||||
|
||||
/**
|
||||
* Styles Loader message fragments without changing their visible text or width.
|
||||
* Set `animated` for colorizers whose ANSI output changes over time.
|
||||
*/
|
||||
export type LoaderMessageColorFn = ColorFn & {
|
||||
readonly animated?: true;
|
||||
};
|
||||
|
||||
/** Animates a spinner and colorized message while asynchronous work is pending. */
|
||||
export class Loader extends Text {
|
||||
#frames = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"];
|
||||
#currentFrame = 0;
|
||||
#intervalId?: NodeJS.Timeout;
|
||||
#ui: TUI | null = null;
|
||||
#lastSpinnerTick = 0;
|
||||
#layoutSource?: readonly string[];
|
||||
#layout?: readonly { leading: string; content: string; trailing: string }[];
|
||||
|
||||
constructor(
|
||||
ui: TUI,
|
||||
@@ -40,11 +41,36 @@ export class Loader extends Text {
|
||||
}
|
||||
|
||||
render(width: number): readonly string[] {
|
||||
const lines = ["", ...super.render(width)];
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
if (visibleWidth(line) > width) {
|
||||
lines[i] = sliceByColumn(line, 0, width, true);
|
||||
const source = super.render(width);
|
||||
if (source !== this.#layoutSource) {
|
||||
const paddingX = getPaddingX(1);
|
||||
this.#layoutSource = source;
|
||||
this.#layout = source.map(line => {
|
||||
const clamped = visibleWidth(line) > width ? sliceByColumn(line, 0, width, true) : line;
|
||||
const body = clamped.slice(paddingX);
|
||||
const content = body.trimEnd();
|
||||
return {
|
||||
leading: clamped.slice(0, paddingX),
|
||||
content,
|
||||
trailing: body.slice(content.length),
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
const frame = this.#frames[this.#currentFrame];
|
||||
const lines = [""];
|
||||
const layout = this.#layout ?? [];
|
||||
for (let i = 0; i < layout.length; i++) {
|
||||
const { leading, content, trailing } = layout[i];
|
||||
if (i === 0 && content.startsWith(frame)) {
|
||||
const remainder = content.slice(frame.length);
|
||||
const separator = remainder.startsWith(" ") ? " " : "";
|
||||
const message = remainder.slice(separator.length);
|
||||
lines.push(
|
||||
`${leading}${this.spinnerColorFn(frame)}${separator}${message ? this.messageColorFn(message) : ""}${trailing}`,
|
||||
);
|
||||
} else {
|
||||
lines.push(`${leading}${content ? this.messageColorFn(content) : ""}${trailing}`);
|
||||
}
|
||||
}
|
||||
return lines;
|
||||
@@ -91,8 +117,8 @@ export class Loader extends Text {
|
||||
|
||||
#updateDisplay() {
|
||||
const frame = this.#frames[this.#currentFrame];
|
||||
const text = `${this.spinnerColorFn(frame)} ${this.messageColorFn(this.message)}`;
|
||||
if (this.setText(text) && this.#ui) {
|
||||
const textChanged = this.setText(`${frame} ${this.message}`);
|
||||
if ((textChanged || this.messageColorFn.animated === true) && this.#ui) {
|
||||
// Direct write: a loader tick changes only this component, so the TUI
|
||||
// can update the already-positioned rows without driving the full
|
||||
// compose/prepare/diff pipeline. Lightweight test stubs may not carry
|
||||
|
||||
@@ -134,6 +134,29 @@ describe("Loader component", () => {
|
||||
loader.stop();
|
||||
});
|
||||
|
||||
it("reuses text layout when only animated ANSI styling changes", () => {
|
||||
vi.useFakeTimers();
|
||||
let colorFrame = 0;
|
||||
const ui = { synchronizedOutput: true, requestDirectWrite: vi.fn(), requestComponentRender: vi.fn() };
|
||||
const colorMessage = ((text: string) => `\x1b[3${colorFrame++ % 3}m${text}\x1b[0m`) as LoaderMessageColorFn & {
|
||||
animated: true;
|
||||
};
|
||||
colorMessage.animated = true;
|
||||
const loader = new Loader(ui as unknown as TUI, text => text, colorMessage, "Checking", ["⠸"]);
|
||||
const stringWidth = spyOn(Bun, "stringWidth");
|
||||
|
||||
const initial = loader.render(40);
|
||||
stringWidth.mockClear();
|
||||
vi.advanceTimersByTime(34);
|
||||
const animated = loader.render(40);
|
||||
|
||||
expect(ui.requestDirectWrite).toHaveBeenCalledTimes(2);
|
||||
expect(stringWidth).not.toHaveBeenCalled();
|
||||
expect(initial[1]).not.toBe(animated[1]);
|
||||
expect(visibleWidth(initial[1])).toBe(visibleWidth(animated[1]));
|
||||
loader.stop();
|
||||
});
|
||||
|
||||
it("holds animated message-only frames when synchronized output is unavailable", () => {
|
||||
vi.useFakeTimers();
|
||||
setSystemTime(new Date(1_000));
|
||||
|
||||
Reference in New Issue
Block a user