Feature/accordion widget clean#2974
Conversation
📝 WalkthroughWalkthroughIntroduces an exported ChangesAnimated accordion
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AnimatedAccordionItem
participant AnimatedAccordion
participant Screen
User->>AnimatedAccordionItem: Click item header
AnimatedAccordionItem->>AnimatedAccordion: Notify accordion
AnimatedAccordion->>AnimatedAccordionItem: Close other open items
AnimatedAccordion->>AnimatedAccordionItem: Set target content heights
AnimatedAccordion->>Screen: Render animated stacked layout
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/AnimatedAccordion.ts`:
- Line 63: Update both _renderSelf overrides in AnimatedAccordion to accept
Screen instead of any, and import Screen from `@termuijs/core`. Preserve the
protected visibility and void return type so the overrides match
Widget._renderSelf.
- Around line 114-125: The parent click handler in AnimatedAccordion must no
longer perform exclusive sibling-closing logic; clear that handler so
AnimatedAccordionItem exclusively owns header-click behavior and toggles the
selected item. Move or preserve the exclusive-close behavior in the item’s click
handler, ensuring siblings close before the selected item toggles. Update the
related expectations in packages/ui/src/AnimatedAccordion.test.ts at lines 34-46
to verify the single-owner behavior.
- Around line 11-37: Update AnimatedAccordionItem to be keyboard-operable by
setting focusable to true and adding handleKey(event: KeyEvent) that responds to
lowercase enter and space keys by calling this.toggle() and this.markDirty().
Preserve the existing click handler behavior.
- Around line 65-74: Update the header rendering in AnimatedAccordion using
grapheme-aware, display-width mapping rather than string indices, so each
terminal cell receives the correct character and wide or clustered characters
occupy their full width. Build the mapped header before calling setCell, and use
the caps.unicode fallback values for the open and closed triangle markers
instead of hardcoded symbols.
- Around line 141-145: Update the accordion measurement flow around
_renderSelf() and setTargetHeight() so the desired open content height is
captured separately before item.content.rect.height is overwritten by the
animated spring height. Pass that stable measured height to setTargetHeight(),
retaining the clipped rect only for rendering the current animation state.
🪄 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: 465d1ad8-de1d-430b-98f2-2846433d0762
📒 Files selected for processing (3)
packages/ui/src/AnimatedAccordion.test.tspackages/ui/src/AnimatedAccordion.tspackages/ui/src/index.ts
Description
This PR implements an
AnimatedAccordioncomponent in@termuijs/uithat accepts multiple collapsible sections. It properly leverages@termuijs/motionphysics (stepSpring) to smoothly animate the opening and closing of content panels. To prevent regressing and clashing with the existingAccordionwidget from@termuijs/widgets, it is exposed uniquely asAnimatedAccordion. It properly handles wide unicode graphemes viastringWidth().Related Issue
Closes #2860
Which package(s)?
@termuijs/ui
Type of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)type:design)type:accessibility)type:performance)type:devops)type:security)Checklist
neverrules are respected.bun run buildpasses locally.bun run lintandbun run typecheckpass locally.Summary by CodeRabbit
New Features
Tests