Skip to content

feat(core): add visible focus ring support#696

Open
Pushtikansara wants to merge 1 commit into
Karanjot786:mainfrom
Pushtikansara:feature-focus-ring
Open

feat(core): add visible focus ring support#696
Pushtikansara wants to merge 1 commit into
Karanjot786:mainfrom
Pushtikansara:feature-focus-ring

Conversation

@Pushtikansara
Copy link
Copy Markdown

@Pushtikansara Pushtikansara commented Jun 4, 2026

Description

This PR connects the existing focus system with widget focus state so that focused widgets correctly render their focus ring.
##Changes
-Added focus and blur event handling in App.
-Synchronized widget isFocused state with FocusManager.
-Triggered widget re-rendering when focus changes.
-Added tests covering focus propagation and synchronization behavior.

Related Issue

Closes #325

Which package(s)?

@termuijs/core

Type of Change

  • 🐛 Bug fix (type:bug)
  • ✨ Feature (type:feature)
  • 📝 Docs (type:docs)
  • 🧪 Tests (type:testing)
  • ♻️ Refactor (type:refactor)
  • 🎨 Design / UX (type:design)
  • ♿ Accessibility (type:accessibility)
  • ⚡ Performance (type:performance)
  • 🔧 DevOps / CI (type:devops)
  • 🔒 Security (type:security)

Checklist

  • ⭐ You starred the repo. The needs-star check blocks your merge otherwise.
  • Tests pass locally: bun vitest run
  • Build passes: bun run build
  • Typecheck passes: bun run typecheck
  • You read CONTRIBUTING.md.
  • Your PR title follows type: short description.
  • Widget state mutators call markDirty() (if your change affects rendering).
  • No new any types without an inline comment explaining why.
  • No unrelated refactors bundled into this PR.

GSSoC 2026 Participation

Screenshots / Recordings (UI changes)

N/A

Notes for the Reviewer

This change synchronizes widget focus state with FocusManager events and ensures focused widgets are re-rendered when focus changes. Tests have been added to verify focus state propagation and synchronization behavior.

Summary by CodeRabbit

  • New Features
    • Implemented focus state tracking and propagation at the application level.
    • Focus-aware widgets now correctly reflect and update their state based on focus changes.
    • Focus changes automatically trigger re-renders and ensure proper synchronization with the widget lifecycle.

@github-actions github-actions Bot added area:core @termuijs/core type:testing +10 pts. Tests. type:feature +10 pts. New feature. needs-star PR author has not starred the repo. labels Jun 4, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

Hi @Pushtikansara 👋

Star this repo before your PR merges.

Why? GSSoC 2026 contributors who star get priority review and points credit. After you star, push any commit (or re-run this check). The needs-star label lifts automatically.

Thanks for your contribution to TermUI.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds application-level focus tracking to App. It subscribes to FocusManager events, propagates isFocused state to focus-aware widgets during widget-tree traversal, and triggers re-renders when focus changes. Tests verify focus flag updates, dirty marking, and event synchronization across the widget lifecycle.

Changes

App-level focus tracking and propagation

Layer / File(s) Summary
FocusAwareWidget contract and App focus lifecycle
packages/core/src/app/App.ts
FocusAwareWidget interface defines widgets with id, isFocused, and optional markDirty. App adds _unsubFocus and _unsubBlur fields and subscribes to FocusManager focus/blur events in the constructor. mount() lazily re-establishes subscriptions if missing; unmount() cleans up listeners and nulls the callbacks.
Widget focus state propagation
packages/core/src/app/App.ts
_walkWidget updates isFocused on focus-aware widgets based on focus.currentId. New private methods _handleFocusEvent, _setWidgetFocused, and _isFocusAwareWidget manage focus state updates, mark affected widgets dirty, and trigger requestRender() when focus changes.
Focus integration tests and test widget
packages/core/src/app/App.test.ts
FocusTestWidget mock widget tracks focus state, render counts, and renders a focus marker (F vs -) to its screen. Helper factories createFocusTestRoot and createInteractiveTestOptions support building focusable test trees. Test suite verifies focus flag propagation, dirty/re-render cycles, and focus event synchronization before mount completes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Suggested labels

gssoc:approved, type:feature, area:core, level:intermediate, type:testing

Suggested reviewers

  • Karanjot786

Poem

🐰 Hop, hop! Focus flows so free,
App tracks where the cursor be,
Dirty marks and renders bright,
Widgets glow with focus light,
Tests confirm each little sight!

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR closes issue #296 which requires implementing a usePolling hook in packages/data. However, the actual changes are in packages/core (App.ts and App.test.ts) with no modifications to packages/data and no usePolling hook implementation. The PR must implement the usePolling hook in packages/data per issue #296 requirements: create usePolling.ts, usePolling.test.ts, and export from packages/data/src/index.ts.
Out of Scope Changes check ⚠️ Warning The PR makes changes only to packages/core, but the linked issue #296 requires all work to be confined to packages/data. The scope of this PR does not match the requirements of the linked issue. Refactor the PR to implement usePolling hook in packages/data as specified in issue #296, and ensure no modifications occur outside packages/data per the AGENTS.md rules.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(core): add visible focus ring support' is highly related to the main change in the PR, which connects the focus system with widget focus state for focus ring rendering.
Description check ✅ Passed The PR description includes all required sections with complete information: clear description, linked issue, package identification, type of change, comprehensive checklist completion, and GSSoC participation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

🎉 Thanks for your first PR to TermUI, @Pushtikansara.

Before your PR merges:

  1. Star the repo. Required. The star-check job blocks your merge otherwise.
  2. ✅ All checks green: build, test, typecheck.
  3. 🏷 PR title follows type: short description. Example: fix: handle empty list.
  4. 🔗 Link your closing issue in the description.

GSSoC 2026 points come from labels after merge:

  • gssoc:approved. +50 base points.
  • level:beginner / intermediate / advanced / critical. +20 / +35 / +55 / +80.
  • quality:clean / exceptional. x 1.2 / x 1.5.
  • type:*. Stackable bonus.

Your reviewer responds within 48 hours. Ping @Karanjot786 on Discord for urgent help.

🚀 Welcome to the cohort.

@Karanjot786 Karanjot786 added gssoc:approved Approved PR. Earns +50 base points. quality:clean x 1.2 multiplier. Clean implementation. level:advanced +55 pts. Complex task. labels Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core @termuijs/core gssoc:approved Approved PR. Earns +50 base points. level:advanced +55 pts. Complex task. needs-star PR author has not starred the repo. quality:clean x 1.2 multiplier. Clean implementation. type:feature +10 pts. New feature. type:testing +10 pts. Tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(core): add focus ring indicators

2 participants