Fix Guidelines-experiment 404 noise, duplicate welcome dialog, and unused-preload warnings#301
Conversation
|
@claude please gie it a review |
|
Claude finished @AllTerrainDeveloper's task in 2m 53s —— View job PR #301 Review
Good work overall. All three fixes are logically sound, backward-compatible, and well-documented. A few issues are worth addressing before merge. Fix 1 — Sticky notes: gate boot on the Guidelines experiment ✅The PHP and TS sides are cleanly symmetric. One test isolation issue —
Fix 2 — Welcome dialog appears twice ✅The
Test isolation bug —
update_user_meta( self::$user_id, 'desktop_mode_mode', '1' );but
The tests all pass (they assert public function test_hidden_once_desktop_mode_is_enabled() {
update_user_meta( self::$user_id, 'desktop_mode_mode', '1' );
$this->addTearDown( static function () {
delete_user_meta( self::$user_id, 'desktop_mode_mode' );
} );
$this->assertFalse( desktop_mode_should_show_welcome_dialog(), '...' );
}Fix 3 — Preload warnings ✅
Minor fragility — update_user_meta( self::$user_id, 'desktop_mode_mode', '' );
$this->assertSame( '', $this->capture_hints() );
update_user_meta( self::$user_id, 'desktop_mode_mode', '1' ); // ← not reached on failureIf the assertion fails the restore never runs, leaving subsequent tests in a disabled-DM state. Use Docs ✅The hooks-reference additions are accurate:
The Summary
The welcome dialog test isolation issue is worth fixing before merge — the tests pass today but test the wrong condition for 3 of 5 assertions. Everything else is solid. |
Three independent console-noise / correctness bugs that surface on a normal
desktop-mode boot. None of them broke functionality, but each spammed the
console (and #2 was a visible UX glitch). All three are fixed with their own
tests and docs.
1. Sticky notes: gate boot on the Gutenberg Guidelines experiment
Symptom
Root cause
Sticky notes are backed by Gutenberg's Guidelines experiment — the
wp_guidelineCPT andwp_guideline_typetaxonomy (REST:wp/v2/guidelinesand
wp/v2/wp_guideline_type). That experiment is opt-in (Gutenberg plugin22.7+, under Gutenberg → Experiments — verified against
lib/experimental/guidelines/class-gutenberg-guidelines-post-type.php). Whenit isn't active those routes 404. The PHP Heartbeat handler already guarded on
post_type_exists()/taxonomy_exists(), but the client booted stickynotes unconditionally and fired the term-resolution probes (and even
fell through to
POSTs trying to create the terms) every load.The errors were already swallowed in JS, so nothing crashed — but the browser
logs every failed request regardless, so the console filled with 404s on every
boot.
Fix
Decide availability once on the server and gate the client boot on it:
includes/sticky-notes/heartbeat.php— newdesktop_mode_sticky_notes_is_available()(
post_type_exists('wp_guideline') && taxonomy_exists('wp_guideline_type')),filterable via the new
desktop_mode_sticky_notes_availablefilter; theHeartbeat guard now reuses it.
includes/render/assets.php— shell config carriesstickyNotes.available.src/types.ts—DesktopConfig.stickyNotes?: { available: boolean }.src/sticky-notes/layer.ts—boot()returns early whenavailable === false(defaults to
trueso an older shell keeps the prior boot-and-swallow path).src/desktop.ts— passesavailable: config.stickyNotes?.availabletobootStickyNotes.No experiment ⇒ no boot ⇒ zero REST probes. With it on (Gutenberg plugin or
a future Core release —
post_type_existscovers both), behavior is unchanged.2. Welcome dialog appears twice after "Enable it now"
Symptom
The "Welcome to Desktop Mode" promo (classic admin) reappears immediately after
the user clicks through it to switch into Desktop Mode.
Root cause
includes/welcome-dialog.phpis a switch-to-Desktop-Mode promo. Its gatesexcluded chromeless iframes but not the desktop shell itself, nor "already
enabled". Clicking "Enable it now":
The redirect won the race, so the "seen" write often hadn't landed. The shell's
parent page is
is_admin()and not chromeless, so the dialog's gate passedagain → it rendered a second time inside the shell.
Fix (
includes/welcome-dialog.php)desktop_mode_is_enabled()is true — it's aswitch-to promo, irrelevant in the shell. This is the robust fix; it no longer
depends on the dismissal write winning the redirect race.
keepalive: trueon the dismissalfetchso the "seen" POSTsurvives the navigation (keeps classic-admin seen-state correct too).
3. "Resource was preloaded but not used in time" warnings
Symptom
Root cause — two distinct issues; a
<link rel="preload">only counts as"used" when the exact same URL (including
?ver=) is requested within ~3s:desktop.css—?ver=mismatch. The preload hint stamped the URL withfilemtime(?ver=1780824029), but the stylesheet handle was registered withthe plugin version (
?ver=0.10.0). Two different URLs for the same file →preload never matched the actual
<link>.shell-overlays.js(andwindow-system.js) — wrong hint type. These arelazy bundles
<script>-injected after first paint (overlays load on thefirst toast / dialog / context-menu), routinely past the ~3s
preloadwindow.preloadis a "use-me-now" contract; these are "use-me-later."Fix
includes/assets.php— registerdesktop.csswith$built_version()(
filemtime) so the stylesheet URL matches the preload hint exactly (andCSS edits cache-bust properly), mirroring
windows.css/chromeless.css.includes/render/assets.php—desktop_mode_print_preload_hints()now honorsa per-hint
rel(preloaddefault, validated; unknown →preload). The twolazy bundles are emitted as
prefetch(same early low-priority cache fill,no must-use-now contract);
desktop.js+desktop.cssstaypreload.Docs
docs/hooks-reference.md— newdesktop_mode_sticky_notes_availablefilter;documented the previously-undocumented
desktop_mode_preload_hints(nowrel-aware) anddesktop_mode_deferred_stylesfilters.docs/javascript-reference.md—desktopModeConfig.stickyNotes.available.docs/architecture.md— note the sticky-notes availability gating.Tests
tests/phpunit/tests/stickyNotesHeartbeat.php— +4:is_availabletrue/false,filter override, and the Heartbeat short-circuit when the surface is absent.
tests/phpunit/tests/welcomeDialog.php— new: shows for a fresh user; hiddenonce enabled (the regression), after dismissal, by filter, and outside admin.
tests/phpunit/tests/preloadHints.php— new: thepreload/prefetchsplitand the
desktop.cssversion match.tests/vitest/sticky-notes.test.ts— +2: no network whenavailable: false;still probes by default.
Verification
php -lclean on all changed PHP.npm run build✓ ·npm run typecheck✓ ·npm run lint✓npm run test:js→ 1555 passednpm run test:php(full suite) → 938 tests / 2194 assertions, OKNotes
stickyNotesconfig flag defaults to "available"when absent; the preload
reldefaults topreload; no documented Stablesignature changed incompatibly.
into three PRs (sticky-notes guard / welcome-dialog / preload hints); they
touch disjoint code paths.
that hid these fixes during testing, the
:8889Core-checkout dev host nowbind-mounts this repo directly at the plugin path via a
docker-compose.override.ymlin thewordpress-alcazabarepo. That filelives in the other repo (local-env only) and is intentionally not part of
this PR.