Fix menu open refresh delay - #1398
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bd187d0d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "storeRefreshing": self.store.isRefreshing ? "1" : "0", | ||
| ]) | ||
| #endif | ||
| self.deferMenuInteractionRefreshIfNeeded() |
There was a problem hiding this comment.
Avoid queuing duplicate refreshes after cached opens
When a background store update has already completed while the menu was closed, it invalidates with allowStaleContentDuringDataRefresh and leaves menuVersion < menuContentVersion but menuVersion >= latestRequiredMenuRebuildVersion; opening that cached menu now enters this preservation path even though no refresh is in flight. This unconditional call sets deferredMenuInteractionRefreshPending, so menuDidClose later schedules scheduleDeferredMenuInteractionRefreshIfNeeded, which calls performStoreRefresh again despite the async open-menu rebuild already rendering the fresh data. In the common “background refresh completed, user opens then closes menu” path this causes an unnecessary second provider refresh/network scrape.
Useful? React with 👍 / 👎.
9bd187d to
0174897
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01748970b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let stillNeedsRetry = retryProviders.contains { | ||
| self.store.isStale(provider: $0) || self.store.snapshot(for: $0) == nil | ||
| } | ||
| if !stillNeedsRetry { | ||
| self.clearSatisfiedDeferredMenuInteractionRefreshes(for: retryProviders) |
There was a problem hiding this comment.
Preserve deferred retry after failed coalesced refresh
When the menu opens while a full store refresh is already in progress, the call to deferMenuInteractionRefreshIfNeeded above is a no-op because that helper returns while store.isRefreshing is true. If the coalesced provider refresh then finishes with an error or still has no snapshot, stillNeedsRetry is true here, but this branch only clears on success and never records the pending retry after the in-flight refresh has ended. In that scenario, closing the menu no longer schedules the retry that the stale/missing card needs, leaving the user stuck until the next timer/manual refresh.
Useful? React with 👍 / 👎.
|
Landed in Verification:
Caveat: the installed Thanks @joshuavial! |
Summary
Tests
18 May 2027while the test expectsMay 18, 2027)Bead: xc-htvhl (RCA + fix the menu-bar click delay (instant cached render, async refresh))