Skip to content

fix: complete PowerPoint slide auto-detection#25

Merged
EinfachMxrc merged 3 commits into
mainfrom
fix/office-init-timing
Apr 10, 2026
Merged

fix: complete PowerPoint slide auto-detection#25
EinfachMxrc merged 3 commits into
mainfrom
fix/office-init-timing

Conversation

@EinfachMxrc

@EinfachMxrc EinfachMxrc commented Apr 10, 2026

Copy link
Copy Markdown
Owner

Probleme & Fixes

1. Office.js Timing-Bug (Hauptursache)

isOfficeAvailable() prüft Office.context — das war noch undefined wenn der useEffect lief. Dadurch wurde officeDetected = false gesetzt und initOfficeBridge nie aufgerufen, obwohl das Add-in in PowerPoint geöffnet war.

Fix: Nur typeof Office !== "undefined" prüfen (Script geladen?), dann initOfficeBridge aufrufen. officeDetected wird erst nach Office.onReady() gesetzt — der einzige Zeitpunkt, wo sicher feststeht, dass wir wirklich in PowerPoint sind.

2. getSelectedDataAsync funktioniert nicht im Taskpane

Die alte API schlägt fehl, wenn der Taskpane-Fokus aktiv ist.

Fix: PowerPoint.run() + getSelectedSlides() (PowerPointApi 1.5) als primäre Methode — fokusunabhängig. Legacy-API als Fallback.

Ergebnis

  • Aktuelle Folie wird automatisch erkannt
  • Gesamtanzahl der Folien wird korrekt angezeigt
  • Kein manuelles Setzen mehr nötig

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support for the modern PowerPoint integration with automatic fallback to legacy methods for wider compatibility.
  • Bug Fixes

    • More reliable Office/PowerPoint detection and readiness reporting during add-in initialization.
    • Faster and more responsive slide-sync behavior.
    • Improved lifecycle cleanup to avoid stale state after teardown.

EinfachMxrc and others added 2 commits April 11, 2026 01:53
getSelectedDataAsync(CoercionType.SlideRange) fails when the taskpane
has focus, so it could never detect the current slide or total count.

Replace with PowerPoint.run() + getSelectedSlides() (PowerPointApi 1.5)
which works independently of keyboard focus. Also fixes slide count
detection since PowerPoint.slides.items.length is always available.

Legacy getSelectedDataAsync path kept as fallback for older Office builds
that don't support PowerPointApi 1.5.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
isOfficeAvailable() checks Office.context which may not be set up yet
when the useEffect fires. This caused officeDetected=false and the
bridge was never initialized even inside PowerPoint.

Fix: check only typeof Office !== "undefined" (script loaded) before
calling initOfficeBridge. officeDetected is now set after Office.onReady
resolves, which is the correct time to know we are truly in PowerPoint.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Apr 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
handout-powerpoint-addin Ready Ready Preview, Comment Apr 10, 2026 11:57pm

@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46e2d138-b2a0-443d-a8fb-b95e4d8d453d

📥 Commits

Reviewing files that changed from the base of the PR and between 64fbe4d and b976396.

📒 Files selected for processing (1)
  • apps/web/src/components/powerpoint/PowerPointAddinClient.tsx

📝 Walkthrough

Walkthrough

Modified PowerPoint add-in detection and bridge logic: replaced prior Office availability gating with direct global Office presence checks, changed bridge initialization to reflect sync capability, added modern PowerPoint.run path with legacy Office API fallback, adjusted polling/debounce timings, and tightened lifecycle teardown.

Changes

Cohort / File(s) Summary
Addin client — Office detection & state
apps/web/src/components/powerpoint/PowerPointAddinClient.tsx
Replaced isOfficeAvailable() with typeof globalThis.Office !== "undefined" checks. officeDetected now set to false immediately when Office script is missing (or token/selectedSessionId absent, or isDemo true) and updated after initOfficeBridge() resolves based on returned sync mode (mode !== "manual_only"). Minor control-flow cleanups in handlers.
Office bridge — slide retrieval, API gating, timers
apps/web/src/lib/powerpoint/officeBridge.ts
Added isPowerPointApiAvailable() to prefer modern PowerPoint.run path; getCurrentSlideInfo() now tries modern getSelectedSlides() & maps slide id → number, with legacyGetSlideInfo() fallback using getSelectedDataAsync(Office.CoercionType.SlideRange) and getSlideCountAsync. Adjusted timings: DEBOUNCE_MS 250→150, POLL_MS 1000→800. Simplified control flow and teardown: destroyOfficeBridge() clears timers/state and sets isInitialized = false.

Sequence Diagram(s)

sequenceDiagram
    participant Client as PowerPointAddinClient
    participant Bridge as officeBridge
    participant ModernAPI as PowerPoint.run
    participant LegacyAPI as Office API

    Client->>Bridge: initOfficeBridge(callbacks)
    activate Bridge
    Bridge->>Bridge: isPowerPointApiAvailable()?
    alt modern API available
        Bridge->>ModernAPI: PowerPoint.run(context => getSelectedSlides())
        activate ModernAPI
        ModernAPI-->>Bridge: selected slide id(s) & slides list
        deactivate ModernAPI
        Bridge->>Bridge: map slide id -> slideNumber
    else fallback
        Bridge->>LegacyAPI: getSelectedDataAsync(SlideRange)
        activate LegacyAPI
        LegacyAPI-->>Bridge: slide range data
        LegacyAPI->>LegacyAPI: getSlideCountAsync() (optional)
        deactivate LegacyAPI
        Bridge->>Bridge: build slide info from legacy data
    end
    Bridge-->>Client: resolve SyncCapability (mode)
    deactivate Bridge
    Client->>Client: set officeDetected = (mode !== "manual_only")
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

Poem

🐰 I hopped in, sniffed the Office sky,

If modern APIs bloom, I’ll try,
If they stumble, I’ll gently fall back—
Slides stay in sync down every track,
A tiny rabbit, bridging the slide stack.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: complete PowerPoint slide auto-detection' directly and clearly summarizes the main changes addressing Office.js timing and slide detection issues in the PowerPoint addin client.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/office-init-timing

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

typeof Office causes a TS error in files without declare const Office.
Use typeof (globalThis as any).Office instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@EinfachMxrc EinfachMxrc merged commit 2cb65a1 into main Apr 10, 2026
2 checks passed
@greptile-apps

greptile-apps Bot commented Apr 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two related issues with PowerPoint slide auto-detection in the Office Add-in: (1) a timing problem where Office.context was undefined when the React useEffect first ran, and (2) getSelectedDataAsync failing when the taskpane held focus. The component-side guard is correctly changed from isOfficeAvailable() to typeof Office !== \"undefined\", and the bridge now uses the modern PowerPoint.run + getSelectedSlides (PowerPointApi 1.5) as the primary method with the legacy API as a fallback.

  • PowerPointAddinClient.tsx: Pre-bridge check now uses only typeof Office !== \"undefined\"; officeDetected is set to true only after Office.onReady() confirms the host is PowerPoint.
  • officeBridge.ts: getCurrentSlideInfo now attempts PowerPoint.run with getSelectedSlides first, falling back to getSelectedDataAsync + getSlideCountAsync; polling at 800 ms is retained as a safety net.
  • Incomplete fix: initOfficeBridge still calls isOfficeAvailable() (which requires Office.context) before reaching Office.onReady(). If Office.context is undefined at call time \u2014 the exact race the PR targets \u2014 the function still returns \"manual_only\" immediately and Office.onReady() is never invoked. The guard on line 44 of officeBridge.ts should be changed to typeof Office === \"undefined\" to complete the fix.

Confidence Score: 3/5

The PR partially fixes the Office.js timing race but leaves the root guard inside initOfficeBridge unchanged, meaning the bug can still reproduce.

The component-level guard change and the new PowerPoint.run/getSelectedSlides primary path are sound improvements. However, initOfficeBridge still calls isOfficeAvailable() (requiring Office.context) before Office.onReady(), which means the stated fix for the timing race can still be defeated in the exact scenario described in the PR. That one-line change on line 44 is the critical missing piece.

apps/web/src/lib/powerpoint/officeBridge.ts line 44 — the isOfficeAvailable() guard in initOfficeBridge must be replaced with typeof Office === undefined to complete the timing fix.

Important Files Changed

Filename Overview
apps/web/src/lib/powerpoint/officeBridge.ts Adds PowerPoint.run + getSelectedSlides as the primary slide-detection method with legacy API fallback and polling; but the timing fix is incomplete — initOfficeBridge still gates on isOfficeAvailable() (which requires Office.context) before calling Office.onReady(), so the root race condition can persist.
apps/web/src/components/powerpoint/PowerPointAddinClient.tsx Correctly changes the pre-bridge guard from isOfficeAvailable() to typeof Office !== "undefined", and defers officeDetected to after Office.onReady() resolves; the component-side fix is sound but depends on the bridge's internal guard also being updated.

Sequence Diagram

sequenceDiagram
    participant Component as PowerPointAddinClient
    participant Bridge as officeBridge.ts
    participant Office as Office.js Runtime

    Component->>Component: useEffect fires
    Component->>Component: typeof Office !== undefined? Yes
    Component->>Bridge: initOfficeBridge(callbacks)
    Bridge->>Bridge: isOfficeAvailable()? checks Office.context too
    alt Office.context still undefined
        Bridge-->>Component: resolve(manual_only) bug not fully fixed
    else Office.context already defined
        Bridge->>Office: Office.onReady()
        Office-->>Bridge: host PowerPoint
        Bridge->>Bridge: isInitialized = true
        Bridge->>Bridge: syncCurrentSlide + startPolling
        Bridge->>Office: addHandlerAsync DocumentSelectionChanged
        Office-->>Bridge: success
        Bridge-->>Component: resolve(auto)
        Component->>Component: setOfficeDetected(true)
    end
    loop Poll every 800ms
        Bridge->>Office: PowerPoint.run getSelectedSlides
        alt API available
            Office-->>Bridge: slide ids
            Bridge->>Component: onSlideChange
        else Fallback
            Bridge->>Office: getSelectedDataAsync
            Office-->>Bridge: slide index
            Bridge->>Office: getSlideCountAsync
            Office-->>Bridge: totalSlides
            Bridge->>Component: onSlideChange
        end
    end
    Component->>Bridge: destroyOfficeBridge
    Bridge->>Bridge: clearInterval + clearTimeout
    Bridge->>Office: removeHandlerAsync
    Bridge->>Bridge: isInitialized = false
Loading

Comments Outside Diff (3)

  1. apps/web/src/lib/powerpoint/officeBridge.ts, line 44-47 (link)

    P1 isOfficeAvailable() guard still blocks Office.onReady() call

    The PR's stated fix is to stop checking Office.context at startup (because it may be undefined when the effect first runs) and instead let Office.onReady() handle initialization. However, initOfficeBridge still calls isOfficeAvailable() at line 44, which checks both typeof Office !== "undefined" and typeof Office.context !== "undefined". If Office.context is still undefined at the time initOfficeBridge is called (the exact race condition being fixed), this guard short-circuits and the function returns "manual_only" immediately — Office.onReady() is never invoked.

    The path through the code:

    1. Component checks typeof Office !== "undefined" → ✅ Office script is loaded
    2. Component calls initOfficeBridge
    3. initOfficeBridge checks isOfficeAvailable() → ❌ Office.context still undefined → returns "manual_only"
    4. setOfficeDetected(false) → same outcome as before the fix

    The guard on line 44 should only check whether the Office global is defined, then rely on Office.onReady() to wait for full initialization:

    getCurrentSlideInfo and destroyOfficeBridge correctly continue to use isOfficeAvailable() because they run after Office.onReady() has already fired (guarded by isInitialized).

    Fix in Codex

  2. apps/web/src/components/powerpoint/PowerPointAddinClient.tsx, line 246-287 (link)

    P1 syncSlide closure captures stale values when slide events fire

    syncSlide is defined inline at the component level (lines 219–244) and closes over token, selectedSessionId, isDemo, and setCurrentSlide. The onSlideChange callback passed to initOfficeBridge captures syncSlide at the time the effect runs. Because the Office bridge lives across many renders (only re-initialized when token, selectedSessionId, isDemo, or setCurrentSlide change), the captured syncSlide from a prior render would call setCurrentSlide with stale token or sessionId if those change for any reason other than the ones in the dep array.

    In practice the dep array does cover token and selectedSessionId, so the bridge is torn down and rebuilt on those changes — this means the bug window is narrow. But the design is fragile. Wrapping syncSlide in useCallback with the same dependencies and including it in the effect's dependency array would make the relationship explicit and safe.

    Fix in Codex

  3. apps/web/src/lib/powerpoint/officeBridge.ts, line 17-21 (link)

    P2 Module-level mutable state limits multiple bridge instances

    isInitialized, activeCallbacks, lastReportedSlide, debounceTimer, and pollTimer are module-level singletons. This works fine as long as only one bridge is ever active, and destroyOfficeBridge is reliably called before the next initOfficeBridge. The current useEffect cleanup does call destroyOfficeBridge, so this is safe today.

    However, the pattern is brittle in the face of React Strict Mode (which intentionally double-invokes effects). Consider wrapping this state in a per-call object or at minimum adding an isInitialized guard at the top of initOfficeBridge to prevent a second simultaneous initialization.

    Fix in Codex

Fix All in Codex

Reviews (1): Last reviewed commit: "fix: use globalThis cast to avoid TS err..." | Re-trigger Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant