Skip to content

Fix Guidelines-experiment 404 noise, duplicate welcome dialog, and unused-preload warnings#301

Merged
AllTerrainDeveloper merged 2 commits into
trunkfrom
fix/errors-sticky-admin-dialogs-css-issues
Jun 7, 2026
Merged

Fix Guidelines-experiment 404 noise, duplicate welcome dialog, and unused-preload warnings#301
AllTerrainDeveloper merged 2 commits into
trunkfrom
fix/errors-sticky-admin-dialogs-css-issues

Conversation

@AllTerrainDeveloper
Copy link
Copy Markdown
Collaborator

@AllTerrainDeveloper AllTerrainDeveloper commented Jun 7, 2026

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

GET /wp-json/wp/v2/wp_guideline_type?context=edit&slug=artifact&per_page=100  404 (Not Found)
GET /wp-json/wp/v2/wp_guideline_type?...&slug=sticky...                        404 (Not Found)
POST /wp-json/wp/v2/wp_guideline_type                                          404 (Not Found)
…

Root cause
Sticky notes are backed by Gutenberg's Guidelines experiment — the
wp_guideline CPT and wp_guideline_type taxonomy (REST: wp/v2/guidelines
and wp/v2/wp_guideline_type). That experiment is opt-in (Gutenberg plugin
22.7+, under Gutenberg → Experiments — verified against
lib/experimental/guidelines/class-gutenberg-guidelines-post-type.php). When
it isn't active those routes 404. The PHP Heartbeat handler already guarded on
post_type_exists() / taxonomy_exists(), but the client booted sticky
notes 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 — new desktop_mode_sticky_notes_is_available()
    (post_type_exists('wp_guideline') && taxonomy_exists('wp_guideline_type')),
    filterable via the new desktop_mode_sticky_notes_available filter; the
    Heartbeat guard now reuses it.
  • includes/render/assets.php — shell config carries stickyNotes.available.
  • src/types.tsDesktopConfig.stickyNotes?: { available: boolean }.
  • src/sticky-notes/layer.tsboot() returns early when available === false
    (defaults to true so an older shell keeps the prior boot-and-swallow path).
  • src/desktop.ts — passes available: config.stickyNotes?.available to bootStickyNotes.

No experiment ⇒ no boot ⇒ zero REST probes. With it on (Gutenberg plugin or
a future Core release — post_type_exists covers 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.php is a switch-to-Desktop-Mode promo. Its gates
excluded chromeless iframes but not the desktop shell itself, nor "already
enabled"
. Clicking "Enable it now":

  1. fired a fire-and-forget "mark intro seen" POST, then
  2. immediately redirected into the shell.

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 passed
again → it rendered a second time inside the shell.

Fix (includes/welcome-dialog.php)

  • Add a gate: never show once desktop_mode_is_enabled() is true — it's a
    switch-to promo, irrelevant in the shell. This is the robust fix; it no longer
    depends on the dismissal write winning the redirect race.
  • Defensive: keepalive: true on the dismissal fetch so the "seen" POST
    survives the navigation (keeps classic-admin seen-state correct too).

3. "Resource was preloaded but not used in time" warnings

Symptom

The resource …/assets/js/shell-overlays.js?ver=… was preloaded using link
preload but not used within a few seconds from the window's load event.
The resource …/assets/css/desktop.css?ver=… was preloaded …

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 with
    filemtime (?ver=1780824029), but the stylesheet handle was registered with
    the plugin version (?ver=0.10.0). Two different URLs for the same file →
    preload never matched the actual <link>.
  • shell-overlays.js (and window-system.js) — wrong hint type. These are
    lazy bundles <script>-injected after first paint (overlays load on the
    first toast / dialog / context-menu), routinely past the ~3s preload window.
    preload is a "use-me-now" contract; these are "use-me-later."

Fix

  • includes/assets.php — register desktop.css with $built_version()
    (filemtime) so the stylesheet URL matches the preload hint exactly (and
    CSS edits cache-bust properly), mirroring windows.css / chromeless.css.
  • includes/render/assets.phpdesktop_mode_print_preload_hints() now honors
    a per-hint rel (preload default, validated; unknown → preload). The two
    lazy bundles are emitted as prefetch (same early low-priority cache fill,
    no must-use-now contract); desktop.js + desktop.css stay preload.

Docs

  • docs/hooks-reference.md — new desktop_mode_sticky_notes_available filter;
    documented the previously-undocumented desktop_mode_preload_hints (now
    rel-aware) and desktop_mode_deferred_styles filters.
  • docs/javascript-reference.mddesktopModeConfig.stickyNotes.available.
  • docs/architecture.md — note the sticky-notes availability gating.

Tests

  • tests/phpunit/tests/stickyNotesHeartbeat.php — +4: is_available true/false,
    filter override, and the Heartbeat short-circuit when the surface is absent.
  • tests/phpunit/tests/welcomeDialog.php — new: shows for a fresh user; hidden
    once enabled (the regression), after dismissal, by filter, and outside admin.
  • tests/phpunit/tests/preloadHints.php — new: the preload/prefetch split
    and the desktop.css version match.
  • tests/vitest/sticky-notes.test.ts — +2: no network when available: false;
    still probes by default.

Verification

  • php -l clean on all changed PHP.
  • npm run build ✓ · npm run typecheck ✓ · npm run lint
  • npm run test:js1555 passed
  • npm run test:php (full suite) → 938 tests / 2194 assertions, OK

Notes

  • Backward compatible. The stickyNotes config flag defaults to "available"
    when absent; the preload rel defaults to preload; no documented Stable
    signature changed incompatibly.
  • These are three logically separate fixes — reviewers who prefer can split them
    into three PRs (sticky-notes guard / welcome-dialog / preload hints); they
    touch disjoint code paths.
  • Local dev note (not part of this diff): to stop the stale-mirror problem
    that hid these fixes during testing, the :8889 Core-checkout dev host now
    bind-mounts this repo directly at the plugin path via a
    docker-compose.override.yml in the wordpress-alcazaba repo. That file
    lives in the other repo (local-env only) and is intentionally not part of
    this PR.
Open WordPress Playground Preview

@AllTerrainDeveloper
Copy link
Copy Markdown
Collaborator Author

@claude please gie it a review

@claude
Copy link
Copy Markdown

claude Bot commented Jun 7, 2026

Claude finished @AllTerrainDeveloper's task in 2m 53s —— View job


PR #301 Review

  • Gather context and understand request
  • Read PHP changes (heartbeat.php, assets.php, render/assets.php, welcome-dialog.php)
  • Read TypeScript changes (desktop.ts, layer.ts, types.ts)
  • Read test files (phpunit + vitest)
  • Read docs changes
  • Formulate and post 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. desktop_mode_sticky_notes_is_available() is a minimal, filterable check; the heartbeat guard reuses it; the config field carries it down; the layer's constructor defaults to true for old shells. The backward-compat path (config.stickyNotes?.availableundefined?? true in layer constructor) is correct.

One test isolation issue — stickyNotesHeartbeat.php:

test_is_not_available_without_the_guidelines_taxonomy (line 145) and test_heartbeat_skips_delta_when_surface_is_unavailable (line 167) both call unregister_taxonomy() without restoring it. They rely on set_up() re-registering via register_guidelines_surface(). This works fine only if register_guidelines_surface() registers the taxonomy when !taxonomy_exists(). Worth a quick glance to confirm the helper handles both CPT + taxonomy re-registration.


Fix 2 — Welcome dialog appears twice ✅

The desktop_mode_is_enabled() gate at includes/welcome-dialog.php:65 is the right solution — it removes the race-condition dependency entirely. The comment in both PHP and JS accurately explains the prior failure mode (fire-and-forget POST losing to the redirect).

keepalive: true on the dismissal fetch is correct: allows the POST to outlive the navigation, size is negligible, browser support is universal on modern browsers.

Test isolation bug — welcomeDialog.php ⚠️

test_hidden_once_desktop_mode_is_enabled (line 44) calls:

update_user_meta( self::$user_id, 'desktop_mode_mode', '1' );

but tear_down() never clears it. This means every test that runs after this one in the class will also have DM enabled — which causes them to return false via the new gate 4 rather than via the gate they actually claim to test:

  • test_hidden_after_intro_dismissed → still asserts false, but via the DM-enabled gate, not the seen-intro gate
  • test_filter_can_suppress_the_dialog → same — the filter never even fires because the enabled-gate short-circuits first

The tests all pass (they assert false) but they test the wrong thing. Fix: delete the meta inside the test after assertion, or use a dedicated setUp/addTearDown:

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 this →


Fix 3 — Preload warnings ✅

desktop.css version mismatch — Registering the handle with $built_version('assets/css/desktop.css') (filemtime) rather than $version (semver) is the right fix. The inline comment in includes/assets.php:43–54 explaining why the URL must match the hint is excellent.

prefetch for lazy bundles — The logic in desktop_mode_print_preload_hints() is clean: the rel key defaults to preload, only prefetch is a valid override (anything else collapses back to preload). The hint list is correct — critical path gets preload, window-system + shell-overlays get prefetch.

Minor fragility — preloadHints.php test_no_hints_when_desktop_mode_disabled (line 98):

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 failure

If the assertion fails the restore never runs, leaving subsequent tests in a disabled-DM state. Use addTearDown here:

Fix this →


Docs ✅

The hooks-reference additions are accurate:

  • desktop_mode_sticky_notes_available — signature, param, return, example.
  • desktop_mode_preload_hints — updated with rel key and the "must match exact URL" note.
  • desktop_mode_deferred_styles — previously undocumented; now well-described.

The stickyNotes.available field entry in both javascript-reference.md and the $config filter JSDoc in render/assets.php:380 are well-written.


Summary

Area Status
Sticky notes gating (PHP + TS + tests)
Welcome dialog duplicate fix
Welcome dialog test isolation ⚠️ needs fix
Preload hint split (preload vs prefetch)
desktop.css version alignment
Preload test fragility minor nit
Docs
Backward compatibility

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.

@AllTerrainDeveloper AllTerrainDeveloper enabled auto-merge (squash) June 7, 2026 15:54
@AllTerrainDeveloper AllTerrainDeveloper merged commit 6db26d6 into trunk Jun 7, 2026
5 checks passed
@AllTerrainDeveloper AllTerrainDeveloper deleted the fix/errors-sticky-admin-dialogs-css-issues branch June 7, 2026 15:56
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.

1 participant