Feat progress column api#715
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a new Progress widget with a composable ProgressColumn API (Bar/Text/Percentage/Time/Speed), child-derived or prop-provided columns, per-column maxRefresh throttling, rendering and caching logic, tests, and barrel exports. ChangesProgress Widget with Composable ProgressColumn API
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
area:widgets
Contributor
Author
|
Hi @Karanjot786 👋 I've completed the implementation for #96 and submitted the PR. Typecheck and tests are passing. Could you please review it when you have time? Thanks! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/widgets/src/feedback/Progress.ts`:
- Around line 133-146: Throttling currently skips adding a cell when (now - last
< intervalMs), causing columns to drop; replace the continue with logic that
preserves layout by appending a stable placeholder or the previously rendered
cell for that column: fetch a cached cell/value from a per-column cache (e.g.,
this._lastRenderedCells.get(columnIndex) or
this._lastRenderedValues.get(columnIndex)) and append that instead of skipping,
and ensure you update that cache whenever you render a fresh cell so subsequent
throttled frames can reuse it; reference symbols: refreshRate, column.kind,
columnIndex, this._lastRefreshTimes.
- Around line 44-53: Progress has unchecked "any" usages and assertion-heavy
vnode parsing and also mis-handles throttling spacing and Unicode fallback;
change _renderSelf signature and constructor to use Screen and Style types from
`@termuijs/core` instead of any (replace "screen: any" and "super(style as any)" /
"this.style as any" with the imported Screen and Style), replace the ad-hoc
casts in _resolveColumns/_renderSelf vnode handling with a proper type guard
that narrows items to a VNode-like shape (check typeof item === 'object' && item
!== null && 'type' in item and then assert a typed interface), fix the
throttling logic inside the column loop so skipped columns still append a
placeholder to parts (avoid using continue which shortens parts and breaks
parts.join(' ') spacing), and use caps.unicode with an ASCII fallback when
choosing bar characters (use caps.unicode ? '█' : fallbackChar and similarly for
empty/bar gaps) to ensure correct rendering across terminals.
In `@packages/widgets/src/feedback/ProgressColumn.test.ts`:
- Around line 53-68: Update the tests to render the Progress widget into a real
Screen instance: create a Screen (import from `@termuijs/core`), instantiate
Progress with the provided columns (BarColumn, TextColumn) and also with no args
for the default case, call progress.updateRect(...) to set its size/position,
then call progress.render(screen) and assert the screen's buffer/characters
contain the expected output for each case instead of only checking
progress.columns length; reference Progress, BarColumn, TextColumn,
progress.updateRect, and progress.render when locating where to change the
tests.
🪄 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: 5d15f593-87fc-4525-9fdf-5e247f9c943c
📒 Files selected for processing (4)
packages/widgets/src/feedback/Progress.tspackages/widgets/src/feedback/ProgressColumn.test.tspackages/widgets/src/feedback/ProgressColumn.tspackages/widgets/src/index.ts
Comment on lines
+53
to
+68
| it('uses provided columns in Progress', () => { | ||
| const progress = new Progress({ | ||
| columns: [ | ||
| BarColumn(), | ||
| TextColumn(), | ||
| ], | ||
| }); | ||
|
|
||
| expect(progress.columns).toHaveLength(2); | ||
| }); | ||
|
|
||
| it('falls back to default columns', () => { | ||
| const progress = new Progress(); | ||
|
|
||
| expect(progress.columns.length).toBeGreaterThan(0); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the test file and print the relevant section
ls -la packages/widgets/src/feedback/ProgressColumn.test.ts
sed -n '1,140p' packages/widgets/src/feedback/ProgressColumn.test.ts
# Check if the file imports/uses Screen and render/updateRect anywhere
rg -n "Screen|updateRect|render|render\(" packages/widgets/src/feedback/ProgressColumn.test.ts || true
# Find a known-good widget test pattern (Gauge) and inspect it briefly
GAUGE_FILE=$(fd -i -a -t f "Gauge.test\.ts" packages/widgets/src | head -n 1 || true)
echo "Gauge test file: $GAUGE_FILE"
if [ -n "$GAUGE_FILE" ]; then
sed -n '1,220p' "$GAUGE_FILE"
rg -n "Screen|updateRect|render|caps\.unicode" "$GAUGE_FILE" || true
fi
# List any other Progress-related tests for context
fd -i -a "ProgressColumn\.test\.ts" packages/widgets/src | cat
fd -i -a "Progress.*\.test\.ts" packages/widgets/src | head -n 50Repository: Karanjot786/TermUI
Length of output: 5468
Render Progress with a real Screen in widget tests
packages/widgets/src/feedback/ProgressColumn.test.ts (lines 53-68) only checks progress.columns length, so it doesn’t validate any observable rendered output or widget render contract. Update these tests to instantiate a real Screen from @termuijs/core, call progress.updateRect(...), then progress.render(screen), and assert the rendered characters for both the provided columns case and the default columns fallback.
🤖 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/feedback/ProgressColumn.test.ts` around lines 53 - 68,
Update the tests to render the Progress widget into a real Screen instance:
create a Screen (import from `@termuijs/core`), instantiate Progress with the
provided columns (BarColumn, TextColumn) and also with no args for the default
case, call progress.updateRect(...) to set its size/position, then call
progress.render(screen) and assert the screen's buffer/characters contain the
expected output for each case instead of only checking progress.columns length;
reference Progress, BarColumn, TextColumn, progress.updateRect, and
progress.render when locating where to change the tests.
Karanjot786
added
gssoc:approved
Owner
Review: Throttled columns drop layout cellsIn Fix it by caching the last rendered value per column: // add a cache map
private _lastRenderedCells = new Map<number, string>();
// in the render loop instead of continue:
if (now - last < intervalMs) {
const cached = this._lastRenderedCells.get(columnIndex) ?? ' ';
// render cached cell
} else {
// render fresh cell and update cache
this._lastRenderedCells.set(columnIndex, freshCell);
}This keeps layout stable across throttled frames. |
Karanjot786
force-pushed
the
feat-progress-column-api
branch
from
98298b7 to
1415d74
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/widgets/src/feedback/ProgressColumn.ts (1)
34-42: ⚡ Quick winUse spread operator for consistency with other column factories.
TextColumn explicitly destructures
templateandmaxRefresh, while BarColumn, TimeColumn, SpeedColumn, and PercentageColumn use the spread operator. Both approaches work correctly, but the inconsistency reduces maintainability.♻️ Proposed refactor to match the pattern used by other factories
export function TextColumn( props: TextColumnProps = {}, ): ProgressColumnDefinition { return { kind: 'text', - template: props.template, - maxRefresh: props.maxRefresh, + ...props, }; }🤖 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/feedback/ProgressColumn.ts` around lines 34 - 42, The TextColumn factory currently returns an object that explicitly copies template and maxRefresh; change it to use the same spread pattern as the other factories by returning { kind: 'text', ...props } so it accepts any fields defined on TextColumnProps and stays consistent with BarColumn/TimeColumn/SpeedColumn/PercentageColumn; update the return in the TextColumn function to spread props into the ProgressColumnDefinition (keeping kind: 'text') so TextColumnProps and ProgressColumnDefinition usage remains correct.
🤖 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.
Nitpick comments:
In `@packages/widgets/src/feedback/ProgressColumn.ts`:
- Around line 34-42: The TextColumn factory currently returns an object that
explicitly copies template and maxRefresh; change it to use the same spread
pattern as the other factories by returning { kind: 'text', ...props } so it
accepts any fields defined on TextColumnProps and stays consistent with
BarColumn/TimeColumn/SpeedColumn/PercentageColumn; update the return in the
TextColumn function to spread props into the ProgressColumnDefinition (keeping
kind: 'text') so TextColumnProps and ProgressColumnDefinition usage remains
correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0777b96a-9b81-4e7c-ad1c-decc4019c9a5
📒 Files selected for processing (4)
packages/widgets/src/feedback/Progress.tspackages/widgets/src/feedback/ProgressColumn.test.tspackages/widgets/src/feedback/ProgressColumn.tspackages/widgets/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/widgets/src/feedback/ProgressColumn.test.ts
- packages/widgets/src/index.ts
- packages/widgets/src/feedback/Progress.ts
Owner
|
Thank you so much for your contribution, @Komal2008! Your PR #715 added the ProgressColumn API to @termuijs/widgets. Developers can now show per-row progress bars inside DataGrid columns. Keep contributing. We appreciate your work. |
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
2 participants
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
Adds a composable ProgressColumn API for the Progress widget. This introduces built-in column definitions (BarColumn, TextColumn, TimeColumn, SpeedColumn, and PercentageColumn), template-based rendering, custom render callback support, and default column fallbacks while maintaining backward compatibility.
Related Issue
Closes #96
Which package(s)?
@termuijs/widgets
Type of Change
type:feature)type:testing)type:refactor)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typechecktype: short description.markDirty()(if your change affects rendering).anytypes without an inline comment explaining why.GSSoC 2026 Participation
https://gssoc.girlscript.org/profile/2af89ea2-51ba-40f3-a887-93e5300bea0eScreenshots
Notes for the Reviewer
Changes Included
Added
ProgressColumn.tswith composable column definitions.Added support for:
BarColumnTextColumnTimeColumnSpeedColumnPercentageColumnAdded template-based text rendering (
{task.status}style templates).Added custom render callback support for columns.
Added column resolution logic with default fallback columns.
Preserved existing Progress usage when no columns are provided.
Added unit tests for ProgressColumn definitions and Progress behavior.
Exported Progress and ProgressColumn APIs through the widgets package index.
Validation
Summary by CodeRabbit
New Features
Tests