quick fix for #2918 (refocus block after magnify button click)

[sawka]

No description provided.

[coderabbitai]

Walkthrough

The change modifies the magnify toggle handler in the blockframe header component to introduce a delayed refocus action. When magnification is toggled, the handler now calls refocusNode(blockId) after a 50-millisecond delay. Import statements were adjusted accordingly, removing getConnStatusAtom and adding refocusNode. Additionally, DurableSessionFlyover JSX elements were reformatted across two locations for readability, with no functional impact.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No description was provided by the author; the pull request relies entirely on the title to convey its purpose.	Add a brief description explaining what the issue was and how the fix works, to provide context for reviewers and future maintainers.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: refocusing a block after the magnify button is clicked, addressing issue #2918.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sawka/fix-magnify-focus

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[cloudflare-workers-and-pages]

Deploying waveterm with    Cloudflare Pages

Latest commit:	d4ee5b4
Status:	✅  Deploy successful!
Preview URL:	https://37a32f17.waveterm.pages.dev
Branch Preview URL:	https://sawka-fix-magnify-focus.waveterm.pages.dev

View logs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/app/block/blockframe-header.tsx`:
- Around line 144-147: The toggleMagnify handler currently calls
nodeModel.toggleMagnify() then schedules refocusNode(blockId) with setTimeout
without tracking or clearing the timer; change it to store the timer id (e.g. a
ref like magnifyTimeoutRef) and clearTimeout before setting a new timer to
debounce rapid toggles, and also clear the timer in the component cleanup
(useEffect return or componentWillUnmount) to avoid invoking refocusNode after
unmount; update the toggleMagnify callback to clear previous magnifyTimeoutRef,
set a new timeout for refocusNode(blockId), and ensure cleanup logic clears that
timeout.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c5f2f9 and d4ee5b4.

📒 Files selected for processing (1)

• frontend/app/block/blockframe-header.tsx

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Avoid stray refocus after unmount or rapid toggles (Line 144–147).

The timeout isn’t cleared; repeated clicks or unmounts can queue multiple refocus calls and potentially jump focus to a closed block. Consider debouncing and cleaning up the timer.

🔧 Suggested fix with timeout tracking & cleanup

 const HeaderEndIcons = React.memo(({ viewModel, nodeModel, blockId }: HeaderEndIconsProps) => {
+    const refocusTimerRef = React.useRef<number | null>(null);
     const endIconButtons = util.useAtomValueSafe(viewModel?.endIconButtons);
     const magnified = jotai.useAtomValue(nodeModel.isMagnified);
@@
+    React.useEffect(() => {
+        return () => {
+            if (refocusTimerRef.current != null) {
+                clearTimeout(refocusTimerRef.current);
+            }
+        };
+    }, []);
@@
-                toggleMagnify={() => {
-                    nodeModel.toggleMagnify();
-                    setTimeout(() => refocusNode(blockId), 50);
-                }}
+                toggleMagnify={() => {
+                    nodeModel.toggleMagnify();
+                    if (refocusTimerRef.current != null) {
+                        clearTimeout(refocusTimerRef.current);
+                    }
+                    refocusTimerRef.current = window.setTimeout(() => refocusNode(blockId), 50);
+                }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/app/block/blockframe-header.tsx` around lines 144 - 147, The
toggleMagnify handler currently calls nodeModel.toggleMagnify() then schedules
refocusNode(blockId) with setTimeout without tracking or clearing the timer;
change it to store the timer id (e.g. a ref like magnifyTimeoutRef) and
clearTimeout before setting a new timer to debounce rapid toggles, and also
clear the timer in the component cleanup (useEffect return or
componentWillUnmount) to avoid invoking refocusNode after unmount; update the
toggleMagnify callback to clear previous magnifyTimeoutRef, set a new timeout
for refocusNode(blockId), and ensure cleanup logic clears that timeout.

[kiloconnect]

Code Review Summary

Status: 1 Issue Found (Outside Diff) | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	0

Other Observations (not in diff)

Issues found in unchanged code that cannot receive inline comments:

File	Line	Issue
frontend/app/block/blockframe-header.tsx	40	WARNING: Context menu magnify handler missing refocus fix. The magnify button (line 144-147) now calls refocusNode(blockId) after toggling, but the context menu handler at line 40 does not. This creates inconsistent behavior - the bug from #2918 will still occur when users magnify/unmagnify via right-click context menu. The same fix should be applied: click: () => { nodeModel.toggleMagnify(); setTimeout(() => refocusNode(blockId), 50); }

Files Reviewed (1 file)

• frontend/app/block/blockframe-header.tsx - Changes look correct; 1 issue in unchanged code

---

Changes in Diff: The PR correctly fixes the focus issue for the magnify button by adding refocusNode(blockId) after toggleMagnify(). The import changes and formatting are appropriate. However, the same fix should be applied to the context menu handler to ensure consistent behavior across all magnify/unmagnify actions.

Fix these issues in Kilo Cloud