Skip to content

Fix extension revealing issues - #1683

Merged
cyanzhong merged 1 commit into
mainfrom
develop
Aug 13, 2026
Merged

Fix extension revealing issues#1683
cyanzhong merged 1 commit into
mainfrom
develop

Conversation

@cyanzhong

Copy link
Copy Markdown
Contributor

No description provided.

@cyanzhong
cyanzhong force-pushed the develop branch 2 times, most recently from 40fb64a to 3deaa74 Compare August 12, 2026 12:58
@cyanzhong
cyanzhong requested a balanced review from Copilot August 12, 2026 12:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves extension row revealing after registry loading and refines highlight animation timing.

Changes:

  • Defers target scrolling until extension loading completes.
  • Synchronizes displayed rows before revealing a target.
  • Adds cancellable fade-in/hold/fade-out highlighting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
ExtensionsWindowController.swift Defers target reveal until loading finishes.
ExtensionsViewController.swift Synchronizes model data before scrolling.
HighlightedText.swift Adds cancellable reveal animation phases.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread MarkEditMac/Sources/Extension/ExtensionsWindowController.swift Outdated
Comment thread MarkEditMac/Modules/Sources/SharedUI/HighlightedText.swift Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (4)

MarkEditMac/Sources/Extension/ExtensionsWindowController.swift:49

  • This task can reveal an obsolete target. If one presentation starts a refresh and another present(scrollTo:) call occurs before it completes, the first invocation still executes this captured target afterward and moves the user away from the newer request. Cancel the prior presentation task or guard it with a presentation generation before applying the reveal.
      if let target {
        prepareToRevealTarget()
        extensionsVC?.reveal(target)

MarkEditMac/Sources/Extension/ExtensionsWindowController.swift:40

  • The early return skips the refresh path whenever the target exists in the cached Discover list. That means a deep link to a cached extension never calls model.load, so registry changes and installed-state reconciliation can remain stale even though the method's refresh contract says every window opening fetches fresh data. Reveal immediately, but still let the refresh task run.
    if let target, canRevealImmediately {
      extensionsVC?.reveal(target)
      return
    }

MarkEditMac/Sources/Extension/ExtensionsViewController.swift:304

  • Setting the clip view's bounds directly bypasses the scroll-range clamping that the previous implementation performed. For a target near the end of the list, rowRect.origin.y - safeAreaInsets.top can exceed the maximum scroll origin and expose a large blank region below the table. Clamp against the clip view's content insets and use its scrolling API.
    scrollView.contentView.setBoundsOrigin(boundsOrigin)

MarkEditMac/Modules/Sources/SharedUI/HighlightedText.swift:52

  • The false transition is ignored, so a row keeps running its private reveal task after it is no longer the model's highlighted item. When a second reveal occurs before the first animation finishes, the old and new rows are highlighted simultaneously. Cancel or fade out the old row when isRevealed becomes false, and align the controller's highlight lifetime with the full animation duration so the normal fade-out is not cut short.
  func revealIfNeeded() {

@cyanzhong
cyanzhong merged commit 8c1c5fe into main Aug 13, 2026
2 checks passed
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.

2 participants