Repository navigation
My Jetpack: Show the highest-priority global notice first - #53400
Conversation
setNotice compared each call against the notice from the last render, so when several watchers set a notice in one effect pass the last caller won and the right notice only replaced it a render later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqfKRQfxuwm723Y1prq8mM
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Backup plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Boost plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Search plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Social plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Protect plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Videopress plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Stats Data plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 1 file.
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqfKRQfxuwm723Y1prq8mM
LiamSarsfield
left a comment
There was a problem hiding this comment.
LGTM. I gave it a try and My Jetpack no longer crashes when two notices land together.
One side effect: with two notices of the same priority, the first one set now wins, where trunk kept the last. So a site with a failed backup and a missing connection now sees the backup notice first. I'm fine with that, but could you say so in the description and the setNotice comment, and add a tie row to the new test?
Could you also add plugin changelog entries, like the last notice fix did? And maybe soften the entry to "Notices: Show the highest-priority notice first when several are set at once.", since a notice that arrives later can still crash until #53004 lands.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqfKRQfxuwm723Y1prq8mM
|
@LiamSarsfield Thanks! Done in f9005a3: the tie behaviour is in the description and the |
…e-priority-stale-state
Fixes MYJP-351
When several notice watchers set a notice in the same render,
setNoticecompared each one against the notice from the previous render, so the last caller won regardless of priority, and the right notice replaced it a render later. For example, the bad-installation notice flashed before "Missing site connection", and each swap re-firedjetpack_myjetpack_global_notice_view.Proposed changes
setNoticedecides priority against the latest queued state, so the highest-priority notice wins on the first render and lower ones are ignored.resetNotice()call insidesetNotice; it was a no-op under React 18 batching, and would make every call win with the new check.setNoticecalls in one effect pass.Related product discussion/links
Does this pull request change what data or activity we track or use?
No new data.
jetpack_myjetpack_global_notice_viewstops firing a second time when a notice is swapped on load.Testing instructions
jp test js packages/my-jetpack.notice-context.test.tsxcovers a lower- and an equal-priority notice set after a high one in the same pass; on trunk both render the second notice.setNoticekeeps changing identity on each notice change, which re-runs the watchers.🤖 Generated with Claude Code
https://claude.ai/code/session_01QqfKRQfxuwm723Y1prq8mM