feat: add smooth expand/collapse animation for Accordion component#1741
Open
pixeltannu wants to merge 2 commits into
Open
feat: add smooth expand/collapse animation for Accordion component#1741pixeltannu wants to merge 2 commits into
pixeltannu wants to merge 2 commits into
Conversation
github-actions
Bot
added
area:widgets
📝 WalkthroughWalkthroughBoth Accordion implementations ( ChangesAccordion and Toast Animation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 7
🧹 Nitpick comments (3)
packages/ui/src/Toast.ts (1)
96-96: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winDebounce animation timer scheduling to avoid callback pileups.
_renderSelfcan be called frequently; scheduling a new 16ms timeout each call while animating can stack redundant callbacks and increase render churn.Suggested fix
export class Toast extends Widget { @@ private _animationMs: number; + 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); + } }🤖 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 96, The setTimeout call in the anyAnimating condition is being scheduled on every _renderSelf invocation, which creates callback pileups when _renderSelf is called frequently. Add debouncing logic to ensure only one timeout is pending at a time by tracking whether a timeout is already scheduled. Before scheduling a new setTimeout that calls this.markDirty(), check if a timeout is already in flight and skip scheduling if one exists. Store the timeout ID in an instance variable and clear it in the callback once it executes so subsequent calls can schedule a new timeout when needed.packages/ui/src/Accordion.ts (1)
24-28: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove unused
visibleLinesfield fromAnimationState.The
visibleLinesfield is assigned in_startAnimation(line 96) but never read—_getVisibleLinesrecalculates visibility from progress each frame. Remove the dead field to avoid confusion.interface AnimationState { startTime: number; opening: boolean; - visibleLines: number; totalLines: number; }And update
_startAnimation:- this._animations.set(index, { - startTime: Date.now(), - opening, - visibleLines: opening ? 0 : totalLines, - totalLines, - }); + this._animations.set(index, { + startTime: Date.now(), + opening, + totalLines, + });🤖 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 24 - 28, Remove the unused `visibleLines` field from the `AnimationState` interface since it is assigned in the `_startAnimation` method but never read—visibility is instead recalculated from progress in each frame by `_getVisibleLines`. Delete the `visibleLines: number;` line from the interface definition and remove any assignment to this field in the `_startAnimation` method to eliminate the dead code.packages/widgets/src/display/Accordion.ts (1)
52-53: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winRemove redundant
caps.unicodeternary—both branches are identical.Both
'>'and'v'are ASCII characters, so the fallback is the same as the primary value. Either use distinct Unicode glyphs with ASCII fallbacks (per coding guidelines), or simply remove the ternary:- this._expandChar = opts.expandChar ?? (caps.unicode ? '>' : '>'); - this._collapseChar = opts.collapseChar ?? (caps.unicode ? 'v' : 'v'); + this._expandChar = opts.expandChar ?? '>'; + this._collapseChar = opts.collapseChar ?? 'v';Alternatively, if you want proper Unicode indicators with ASCII fallback:
- this._expandChar = opts.expandChar ?? (caps.unicode ? '>' : '>'); - this._collapseChar = opts.collapseChar ?? (caps.unicode ? 'v' : 'v'); + this._expandChar = opts.expandChar ?? (caps.unicode ? '▶' : '>'); + this._collapseChar = opts.collapseChar ?? (caps.unicode ? '▼' : 'v');🤖 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/display/Accordion.ts` around lines 52 - 53, The ternary operators in the assignments for _expandChar and _collapseChar are redundant because both the caps.unicode true and false branches return the same value. Either remove the ternary conditions entirely and assign the character directly to both _expandChar and _collapseChar, or if you want to follow the coding guidelines and provide proper Unicode indicators with ASCII fallbacks, replace the assignment values with distinct Unicode glyphs for the true branch while keeping ASCII characters for the false branch.Source: Coding guidelines
🤖 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/Accordion.ts`:
- Around line 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.
- Around line 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.
In `@packages/ui/src/Toast.ts`:
- Around line 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.
- Around line 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.
- 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, ...)`.
In `@packages/widgets/src/display/Accordion.ts`:
- Around line 115-127: In the `_getVisibleLines` method, add an early guard
check to handle when `this._animationMs` is zero or less. Before calculating the
progress value using division by `this._animationMs`, add a condition that
returns the appropriate non-animated visibility (either the section's line count
if the section is open, or 0 if closed) when `this._animationMs` is not a
positive number. This prevents division by zero which would result in Infinity
or NaN values that corrupt the progress calculation.
- Around line 188-196: The _updateHeight() method calculates height based on
_openSet membership, but during animations _renderSelf renders different content
determined by _getVisibleLines(), causing a mismatch between claimed height and
rendered height. In _renderSelf(), after rendering content when anyAnimating is
true, call _updateHeight() to ensure the style height stays synchronized with
the actual number of lines being rendered during the animation.
---
Nitpick comments:
In `@packages/ui/src/Accordion.ts`:
- Around line 24-28: Remove the unused `visibleLines` field from the
`AnimationState` interface since it is assigned in the `_startAnimation` method
but never read—visibility is instead recalculated from progress in each frame by
`_getVisibleLines`. Delete the `visibleLines: number;` line from the interface
definition and remove any assignment to this field in the `_startAnimation`
method to eliminate the dead code.
In `@packages/ui/src/Toast.ts`:
- Line 96: The setTimeout call in the anyAnimating condition is being scheduled
on every _renderSelf invocation, which creates callback pileups when _renderSelf
is called frequently. Add debouncing logic to ensure only one timeout is pending
at a time by tracking whether a timeout is already scheduled. Before scheduling
a new setTimeout that calls this.markDirty(), check if a timeout is already in
flight and skip scheduling if one exists. Store the timeout ID in an instance
variable and clear it in the callback once it executes so subsequent calls can
schedule a new timeout when needed.
In `@packages/widgets/src/display/Accordion.ts`:
- Around line 52-53: The ternary operators in the assignments for _expandChar
and _collapseChar are redundant because both the caps.unicode true and false
branches return the same value. Either remove the ternary conditions entirely
and assign the character directly to both _expandChar and _collapseChar, or if
you want to follow the coding guidelines and provide proper Unicode indicators
with ASCII fallbacks, replace the assignment values with distinct Unicode glyphs
for the true branch while keeping ASCII characters for the false branch.
🪄 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: 6100b617-2bc6-44b4-ae6a-3eeec4384cb5
📒 Files selected for processing (3)
packages/ui/src/Accordion.tspackages/ui/src/Toast.tspackages/widgets/src/display/Accordion.ts
Comment on lines
50
to
58
| 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(); |
There was a problem hiding this comment.
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.
Comment on lines
+101
to
+116
| 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; | ||
| } |
There was a problem hiding this comment.
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.
Comment on lines
+55
to
+58
| 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 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.
Comment on lines
+76
to
+80
| 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 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 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.
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
Comment on lines
+115
to
+127
| private _getVisibleLines(index: number): number { | ||
| const anim = this._animations.get(index); | ||
| if (!anim) { | ||
| return this._openSet.has(index) ? this._sections[index].content.split('\n').length : 0; | ||
| } | ||
| const elapsed = Date.now() - anim.startTime; | ||
| const progress = Math.min(1, elapsed / this._animationMs); | ||
| const visible = anim.opening | ||
| ? Math.floor(progress * anim.totalLines) | ||
| : Math.floor((1 - progress) * anim.totalLines); | ||
| if (progress >= 1) this._animations.delete(index); | ||
| return visible; | ||
| } |
There was a problem hiding this comment.
Guard against division by zero when animationMs is zero.
Same issue as the UI package: if animationMs is 0, division yields NaN or Infinity. Add an early check:
private _getVisibleLines(index: number): number {
const anim = this._animations.get(index);
if (!anim) {
return this._openSet.has(index) ? this._sections[index].content.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/widgets/src/display/Accordion.ts` around lines 115 - 127, In the
`_getVisibleLines` method, add an early guard check to handle when
`this._animationMs` is zero or less. Before calculating the progress value using
division by `this._animationMs`, add a condition that returns the appropriate
non-animated visibility (either the section's line count if the section is open,
or 0 if closed) when `this._animationMs` is not a positive number. This prevents
division by zero which would result in Infinity or NaN values that corrupt the
progress calculation.
Comment on lines
188
to
196
| private _updateHeight(): void { | ||
| let total = this._sections.length; // one title row per section | ||
| let total = this._sections.length; | ||
| for (let i = 0; i < this._sections.length; i++) { | ||
| if (this._openSet.has(i)) { | ||
| total += this._sections[i].content.split('\n').length; | ||
| } | ||
| } | ||
| this._style.height = total; | ||
| } |
There was a problem hiding this comment.
Widget height style does not reflect actual rendered height during animation.
_updateHeight() computes height based on _openSet membership, but during animations, _renderSelf renders a different number of lines (via _getVisibleLines). This mismatch can cause layout jumps in parent containers—the widget claims one height while rendering another.
Consider calling _updateHeight() each render frame with the actual visible line count, or updating the height progressively alongside the animation:
private _updateHeight(): void {
let total = this._sections.length;
for (let i = 0; i < this._sections.length; i++) {
- if (this._openSet.has(i)) {
- total += this._sections[i].content.split('\n').length;
- }
+ total += this._getVisibleLines(i);
}
this._style.height = total;
}Then call this._updateHeight() at the end of _renderSelf when anyAnimating is true to keep the height in sync with rendered content.
🤖 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/display/Accordion.ts` around lines 188 - 196, The
_updateHeight() method calculates height based on _openSet membership, but
during animations _renderSelf renders different content determined by
_getVisibleLines(), causing a mismatch between claimed height and rendered
height. In _renderSelf(), after rendering content when anyAnimating is true,
call _updateHeight() to ensure the style height stays synchronized with the
actual number of lines being rendered during the animation.
Contributor
Author
|
Hi @Karanjot786 , the CI build failure is a pre-existing issue with the Windows/bun setup (tsup not found in @termuijs/core and vitest not resolving). This is unrelated to my changes which are only in:
Could you please verify and merge? Thank you! |
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.
Closes #1733
Changes
animationMsoption toAccordionOptionsAnimationStateinterface for tracking animation_startAnimation()method for expand/collapse effect_getVisibleLines()method for progressive line revealsetTimeoutwith 16ms interval for ~60fps re-renderpackages/uiandpackages/widgetsSummary by CodeRabbit
New Features
animationMsconfiguration parameter