Skip to content

fix: inline comment continuity, unified-mode comment positioning, and macOS media prompt - #215

Merged
huhamhire merged 3 commits into
devfrom
fix/bug-batch
Jul 24, 2026
Merged

fix: inline comment continuity, unified-mode comment positioning, and macOS media prompt#215
huhamhire merged 3 commits into
devfrom
fix/bug-batch

Conversation

@huhamhire

Copy link
Copy Markdown
Owner

A batch of PR-review bug fixes.

1. Keep inline editors alive across a comments/drafts refresh (fix(diff))

A prior fix gave the inline comment zones a reconcile-in-place controller, but two gaps still interrupted an open inline editor on every poll:

  • useDiffComments called setComments() unconditionally, so an unchanged poll still produced a fresh comments array. That churned the mentionCandidates memo, whose identity change re-ran useDraftZones. Now it bails on structural equality (reusing sameCommentList, extracted to a shared commentEquality module) so an unchanged poll keeps the old reference — mirroring what the activity view already does.
  • useDraftZones used the one-shot mount, so a real drafts/comments change tore down and rebuilt every DraftZone root, dropping an open draft editor's edit mode / focus / caret. It now uses the same split structural/content effect + persistent createInlineZones controller as useCommentZones, reconciling draft zones by (side, line) key. A surviving draft re-renders in place; only a removed draft (deleted / published / file switch) unmounts.

The activity timeline already bailed via sameCommentList and needs no change.

2. Anchor inline comments to the actual render mode after auto-degrade (fix(diff))

renderSideBySide threaded through the diff view is the toolbar intent, not the mode Monaco actually renders. When the pane is too narrow Monaco auto-degrades side-by-side → inline while the intent stays true, so old-side (base) items got routed to the original editor Monaco has hidden — a published old-side inline comment then had no visible position, and its glyph dot / overview tick vanished too.

  • Introduces a single source of truth for the actual mode (useActualRenderSideBySide, reading Monaco's .monaco-diff-editor.side-by-side class reactively) and feeds it to useCommentZones / useDraftZones / useLineCommentAdder / useSelectionCapture.
  • useCommentZones remaps old-side glyph/overview decorations onto the modified editor when actually inline (also fixes old-side markers in explicitly-chosen unified — a latent bug).
  • Fixes old-side anchor reveal: the nav anchor dropped side (App/PrPanel/PublishReviewModal/notification) and useDiffNav hardcoded side:'new' with no remap. side is now threaded through the anchor (incl. the notification:activate IPC event) and, at reveal time, the live actual mode decides the target: an old-side reveal uses the original editor only when visible, otherwise remaps the old line onto the modified editor. The cross-file search jump reuses the same path.

3. Stop the unexpected Apple Music / media-library permission prompt on macOS (fix(mac))

On macOS the app popped a "would like to access Apple Music, your music and video activity, and your media library" prompt on launch. Not from packaging (no music entitlement, no NSAppleMusicUsageDescription): it's Chromium's macOS "Now Playing" / media-session integration querying the MediaPlayer framework. The app plays no media, so the permission is unexpected. Disables the MediaSessionService and HardwareMediaKeyHandling features at mac startup (before app.whenReady()).

⚠️ This one is macOS-only and was developed on Windows — needs verification on a macOS build to confirm the prompt no longer appears.

Checks

lint + typecheck (all projects) + build (desktop) pass locally.

huhamhire and others added 3 commits July 24, 2026 11:05
The prior fix (6be1c60) gave the inline comment zones a reconcile-in-place
controller, but two gaps remained that still interrupted an open inline
editor on every poll:

1. useDiffComments called setComments() unconditionally, so an unchanged
   poll still produced a fresh comments array. That churned the
   mentionCandidates memo, whose identity change re-ran useDraftZones. Bail
   on structural equality (reuse sameCommentList, now extracted to a shared
   commentEquality module) so an unchanged poll keeps the old reference and
   downstream memos stay stable — mirroring what the activity view already
   does.

2. useDraftZones used the one-shot mountInlineZones, so a real drafts/
   comments change tore down and rebuilt every DraftZone root, dropping an
   open draft editor's edit mode / focus / caret. Give it the same split
   structural/content effect + persistent createInlineZones controller as
   useCommentZones, reconciling draft zones by (side, line) key. A surviving
   draft re-renders in place (DraftZoneList keys by draft id); only a removed
   draft (deleted / published / file switch) unmounts and runs its cancel
   cleanup.

The now-unused one-shot mountInlineZones wrapper is removed. The activity
timeline already bailed via sameCommentList and needs no change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o-degrade

`renderSideBySide` threaded through the diff view is the toolbar *intent*,
not the mode Monaco actually renders. When the pane is too narrow Monaco
auto-degrades side-by-side → inline (useInlineViewWhenSpaceIsLimited) while
the intent stays true, so old-side (base) items get routed to the original
editor Monaco has hidden. A published old-side inline comment then had no
visible position, and its glyph dot / overview tick vanished too.

Introduce a single source of truth for the *actual* mode
(useActualRenderSideBySide, reading Monaco's `.monaco-diff-editor.side-by-side`
class reactively, the pattern useDiffOverviewMarks already used) and feed it
to every consumer that positions by editor side:

- DiffView passes the actual mode to useCommentZones / useDraftZones /
  useLineCommentAdder / useSelectionCapture (was the raw intent).
- useCommentZones remaps old-side glyph/overview decorations onto the
  modified editor when actually inline (mirroring the zone-body routing in
  computeDesired) — this also fixes old-side markers in explicitly-chosen
  unified, a latent bug.
- useDiffOverviewMarks reuses the shared isActualSideBySide helper.

Also fix old-side anchor reveal, which was broken independently: the nav
anchor dropped `side` (App/PrPanel/PublishReviewModal/notification) and
useDiffNav hardcoded side:'new' with no old→new remap. Thread `side` through
the anchor (incl. the notification:activate IPC event) and, at reveal time,
read the live actual mode: an old-side target reveals on the original editor
only when it is genuinely visible, otherwise remaps the old line onto the
modified editor. The cross-file search jump reuses the same path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…prompt

On macOS the app popped a "would like to access Apple Music, your music and
video activity, and your media library" prompt on launch. It's not from
packaging — no music entitlement and no NSAppleMusicUsageDescription are
declared. It's runtime: Chromium's macOS "Now Playing" / media-session
integration queries the MediaPlayer framework, which triggers the media
library permission. The app plays no media and exposes no now-playing
controls, so the permission is unexpected.

Disable the MediaSessionService and HardwareMediaKeyHandling features (mac
startup, before app.whenReady()) so Chromium never touches the media library.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@huhamhire huhamhire added the bug Something isn't working label Jul 24, 2026
@huhamhire
huhamhire merged commit e159b84 into dev Jul 24, 2026
1 check passed
@huhamhire
huhamhire deleted the fix/bug-batch branch July 24, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant