fix (cimo notice): show correct status after closing the media library #3642
Conversation
WalkthroughIntroduces a module-scoped polling guard to prevent concurrent status polls in the CIMO notice, adds an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI
participant CimoNotice as CimoDownloadNotice
participant pollStatus
participant Server as status check
UI->>CimoNotice: mount (inMediaLibrary? true/false)
alt not in media library
CimoNotice->>pollStatus: start polling
else in media library
CimoNotice-->>pollStatus: skip initial poll
end
pollStatus->>pollStatus: check isPolling
alt already polling
pollStatus-->>CimoNotice: return early
else start polling
pollStatus->>Server: fetch status
Server-->>pollStatus: status response
pollStatus->>CimoNotice: update cimoData/state
alt terminal conditions met (pollOnce / attempts / status transitions)
pollStatus-->>CimoNotice: stop polling, update window.stackable.cimo
else
pollStatus->>pollStatus: schedule next attempt
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/lazy-components/cimo/index.js (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/welcome/useful-plugins.php (1)
289-304: Statusnot_installedis returned without anactionURL, which breaks the "Install now" link after a failed installWhen
user_action === 'install'and the plugin is still not installed, you now return:$response['status'] = 'not_installed';but you never set
$response['action']. On the frontend,pollStatusdoessetData( res.data ), sodata.actionbecomes''. The UI then renders an<a>for the"Install now"state with an emptyhref, so after a failed/aborted install the user sees "Install now" but the link is no longer usable.Two concrete improvements:
- Return a valid install URL when reporting
not_installed(mirroringadd_cimo_args_to_localize_editor):if ( $action === 'install' && ! self::is_plugin_installed( $full_slug ) ) { - $response['status'] = 'not_installed'; + $response['status'] = 'not_installed'; + $response['action'] = html_entity_decode( wp_nonce_url( + add_query_arg( + [ + 'action' => 'install-plugin', + 'plugin' => $slug, + ], + admin_url( 'update.php' ) + ), + 'install-plugin_' . $slug + ) ); } else if ( ! self::is_plugin_activated( $full_slug ) ) {
- Optionally, consider what should happen when
$action === 'activate'but the plugin is actually not installed. Currently this path also falls into the! self::is_plugin_activated()branch and reportsstatus = 'installed', which is misleading; you may want to reportnot_installedthere as well or treat it as an error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lazy-components/cimo/index.js(5 hunks)src/lazy-components/cimo/style.scss(0 hunks)src/welcome/useful-plugins.php(1 hunks)
💤 Files with no reviewable changes (1)
- src/lazy-components/cimo/style.scss
🧰 Additional context used
🧬 Code graph analysis (1)
src/lazy-components/cimo/index.js (1)
src/components/image-control2/index.js (1)
CimoDownloadNotice(87-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: build
🔇 Additional comments (1)
src/lazy-components/cimo/index.js (1)
17-17:inMediaLibraryprop wiring correctly scopes media-frame side effectsThe new
inMediaLibraryprop and its usage look sound:
- Line 17:
const { inMediaLibrary = false } = propskeeps existing callers unchanged.- Lines 102–105: early return from
useEffectwheninMediaLibraryis true prevents re‑patchingwp.media.view.MediaFrame.Selectfrom inside the media library context.- Line 203: media library injection correctly passes
inMediaLibrary={ true }when renderingCimoDownloadNotice.This should avoid double‑binding media frame events while preserving the original behavior for the editor notice.
Also applies to: 102-105, 203-203
Summary by CodeRabbit
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.