packages/slides/src/view/editor/editor.ts (1)

2509-2540: 💤 Low value

Field declaration placement is inconsistent with class structure.

The rotateTooltipEl field is declared between method implementations rather than at the top of the class with other private fields (e.g., lastHoverCursor, renderer, listeners). While valid TypeScript, this makes the field harder to discover when reading the class.

🔧 Suggested location

Move the field declaration to join the other private fields near the top of SlidesEditorImpl:

// Near line 371, with other private fields:
private activeTextEditor: SlidesTextBoxEditor | null = null;
private rotateTooltipEl: HTMLDivElement | null = null;

Then remove the declaration from line 2509 and keep only the methods acquireRotateTooltip and releaseRotateTooltip.

🤖 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/slides/src/view/editor/editor.ts` around lines 2509 - 2540, The
private field rotateTooltipEl is declared mid-class between methods which breaks
consistency; move the declaration "private rotateTooltipEl: HTMLDivElement |
null = null;" up into the block of other private fields (near symbols like
activeTextEditor, lastHoverCursor, renderer, listeners) so all fields are
grouped together, then remove the duplicate declaration currently above
acquireRotateTooltip(), leaving only the methods acquireRotateTooltip() and
releaseRotateTooltip() intact; ensure no other references are changed.

packages/slides/src/view/editor/interactions/keyboard.ts (1)

488-507: 💤 Low value

Minor redundancy: getScope() is called twice.

The scope is checked at line 488 and then re-captured at line 497. Since nothing mutates the scope between these calls, the second call could be avoided by capturing it once.

♻️ Suggested simplification

-      run: (e) => {
-        // If we are drilled into a group, pop one scope level and stop —
-        // do NOT also clear the id-selection so the group itself appears
-        // selected at the parent scope level (matching Google Slides).
-        if (ctx.selection.getScope().length > 0) {
-          e.preventDefault();
+      run: (e) => {
+        // If we are drilled into a group, pop one scope level and stop —
+        // do NOT also clear the id-selection so the group itself appears
+        // selected at the parent scope level (matching Google Slides).
+        const scope = ctx.selection.getScope();
+        if (scope.length > 0) {
+          e.preventDefault();
           // Refit the innermost scoped group so its frame matches the
           // children's current visual extent — children may have moved
           // inside drill-in, leaving `group.frame` stale. The refit
           // preserves the group's rotation and scale (see
           // `worldTightFrame` in `model/group.ts`); only position +
           // dimensions move to wrap the children. Wrapped in `batch`
           // so undo restores the pre-refit state in one step.
-          const scope = ctx.selection.getScope();
           const slideId = ctx.currentSlideId();
-          if (slideId !== undefined && scope.length > 0) {
+          if (slideId !== undefined) {
             const innermost = scope[scope.length - 1];
             ctx.store.batch(() => {
               ctx.store.refitGroup(slideId, innermost);
             });
           }

🤖 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/slides/src/view/editor/interactions/keyboard.ts` around lines 488 -
507, Capture the selection scope once instead of calling
ctx.selection.getScope() twice: store it in a local variable (e.g., const scope
= ctx.selection.getScope()) before the first length check, use that variable for
the subsequent checks and to derive innermost, then proceed to call
ctx.currentSlideId(), ctx.store.batch(() => ctx.store.refitGroup(slideId,
innermost)); remove the redundant second getScope() call and its duplicate
length check so the code uses the single scope variable throughout.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes