Skip to content

Fix per-frame layout jank when focusing a toggle-input-card field#4314

Open
evanpelle wants to merge 1 commit into
mainfrom
fix/toggle-input-card-focus-jank
Open

Fix per-frame layout jank when focusing a toggle-input-card field#4314
evanpelle wants to merge 1 commit into
mainfrom
fix/toggle-input-card-focus-jank

Conversation

@evanpelle

Copy link
Copy Markdown
Collaborator

Problem

Focusing the number field of a toggle-input-card (Game Timer / Gold Multiplier / Starting Gold, in both the single-player and host-lobby modals) cost several ms of layout/paint every frame for as long as the field stayed focused.

Root cause

The input was rendered conditionally${this.checked ? html…… : nothing}. Enabling a toggle therefore freshly inserts the <input> into the DOM, and focusing a just-inserted input is what forced the per-frame layout/paint. An input that was already present in the DOM doesn't do this.

Fix

Keep the input permanently mounted and toggle a hidden class when unchecked, instead of conditionally rendering it. Focusing it is then always focusing an element that was already there. Because both modals share <toggle-input-card>, this single change fixes both.

Also restores the autofocus + select of the field on enable (it had been removed earlier while chasing this bug) — safe now that the input isn't freshly inserted.

No other UX change: the toggle behavior, checkmark, styling, and all three cards behave identically.

Testing

Hard-reload, then in both the Solo and Host-lobby modals, enable each of Game Timer / Gold Multiplier / Starting Gold, type a value, and keep the field focused — smooth, no per-frame jank, and the field autofocuses on enable.

🤖 Generated with Claude Code

The number input in toggle-input-card was rendered conditionally
(`${checked ? html`...<input>...` : nothing}`), so enabling a toggle —
Game Timer / Gold Multiplier / Starting Gold in the single-player and
host-lobby modals — freshly inserted the <input> into the DOM. Focusing
a just-inserted input forces several ms of layout/paint every frame for
as long as it stays focused.

Keep the input permanently mounted and toggle a `hidden` class when
unchecked instead, so focusing it is always focusing an element that was
already in the DOM. Both modals share <toggle-input-card>, so this one
change fixes both.

Also restores autofocus + select of the field on enable (removed earlier
while chasing this bug) — safe now that the input is no longer freshly
inserted.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2115c7f6-5091-4d98-9ebc-9f58a6c108cd

📥 Commits

Reviewing files that changed from the base of the PR and between bb46453 and 622bb0e.

📒 Files selected for processing (1)
  • src/client/components/ToggleInputCard.ts

Walkthrough

ToggleInputCard adds PropertyValues to its lit import and a protected updated() lifecycle hook that auto-focuses and selects the numeric input when checked transitions from false to true. The render() method is updated so the input container is always in the DOM, hidden with CSS when checked is false instead of being removed entirely.

Changes

Auto-focus and always-mounted input in ToggleInputCard

Layer / File(s) Summary
Import, lifecycle hook, and render rework
src/client/components/ToggleInputCard.ts
PropertyValues is added to the lit import. A protected updated() hook detects checked enabling transitions and calls .focus() and .select() on the first input. render() switches from conditional nothing rendering to always mounting the input container with a CSS hidden class when checked is false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • openfrontio/OpenFrontIO#4300: Modifies the same ToggleInputCard.ts file around the same updated() focus/select behavior when checked becomes enabled — one PR adds it back, the other removes it to stop focus-induced per-frame jank.

Poem

🎯 The input stays put, never leaves the stage,
A CSS veil hides it, turn the page.
When the checkbox flips, the cursor flies,
focus() and select() — a pleasant surprise.
Always mounted, always ready to type! ⌨️

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main performance fix — preventing layout jank when focusing a toggle-input-card field by changing how the input element is rendered.
Description check ✅ Passed The description clearly explains the problem (per-frame layout jank), root cause (conditional rendering), and solution (permanent mounting with hidden class toggling).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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


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.

❤️ Share

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant