Skip to content

Pre demo polishment#255

Merged
AllTerrainDeveloper merged 3 commits into
trunkfrom
pre-demo-polishment
May 20, 2026
Merged

Pre demo polishment#255
AllTerrainDeveloper merged 3 commits into
trunkfrom
pre-demo-polishment

Conversation

@AllTerrainDeveloper

@AllTerrainDeveloper AllTerrainDeveloper commented May 20, 2026

Copy link
Copy Markdown
Collaborator

A set of targeted bug fixes and a feature extension to the user activity footprint. No architectural changes; each fix is self-contained and regression-tested.


Fixes

Dock-peek: native-window icons now open correctly (src/dock.ts)

Dock items whose target is a native window (My WordPress, Jorvy, plugin-registered launchers) carry a windowId but no url. Clicking them in dock-peek was falling through to the generic URL-based iframe path, which opened a chrome-only window with src="" — the user saw a title bar with no content.

The fix adds an early guard: if item.windowId && !item.url, the click is routed directly through wp.desktop.openNewWindow(), spawning a proper native instance. If openNewWindow returns falsy (old API, feature flag off) the existing fallback logic handles it transparently.

My WordPress: multiple instances no longer interfere with each other (src/my-wordpress/index.ts)

Three related issues when two My WordPress windows were open at the same time:

  1. Close clobbered live siblings. The previous teardown used a desktop-mode-window-closed document listener matched on the base WINDOW_ID. Every sibling's close event fired the same handler — closing instance B cleared instance A's state and left it broken. Fixed by having renderInto return a per-instance teardown that the framework calls only for that specific window.

  2. Navigation in a non-foreground instance threw. asRenderState compared against the singleton activeState. Drilling into a posts list inside a backgrounded duplicate triggered "host body does not match active state". Fixed by introducing a liveStates: Map<HTMLElement, RenderState> that lets asRenderState resolve the correct state for any live instance.

  3. Drag-rejection drop targets overwrote each other. Both instances registered a claimant with the same id (my-wordpress-reject), so the second registration silently replaced the first's element binding. Fixed by appending a per-mount monotonic counter (rejectIdCounter) to the id.

UI: willReadFrequently: true on pixel-readback canvas contexts (src/ui/components/wpd-crumb-chain/wpd-crumb-chain.ts, src/ui/util/avatar-resolve.ts)

Both wpd-crumb-chain (foreground-color picker) and avatar-resolve (transparent-pixel probe) create a 1×1 canvas and immediately call getImageData. Without the willReadFrequently hint Chrome backs the canvas with a GPU texture, so each getImageData triggers a GPU→CPU sync. The breadcrumb chain re-paints on every navigation; the avatar resolver fires once per unique URL across the page — often enough that Chrome was surfacing the "willReadFrequently" performance warning in both cases.


Feature: User activity footprint now tracks post updates / revisions

includes/my-wordpress/user-footprint.php · src/my-wordpress/index.ts · src/my-wordpress/types.ts

The activity heatmap and stats previously only counted published posts and comments. Editors who spend most of their time updating existing content (the typical CMS workflow) appeared to have no activity. This brings the footprint model in line with how GitHub counts contributions — edits to existing content are first-class activity.

What counts as an "update": a WordPress revision saved by the user after the parent post's initial creation (r.post_date_gmt > p.post_date_gmt). The initial save is excluded so it doesn't double-count with the "posts" column. Updates are attributed to whoever hit Save (via r.post_author), not the post owner — consistent with tracking editorial work across a team.

Changes in detail:

  • daily[].updates — per-day update count, added alongside the existing posts and comments fields (zero-filled for days with no saves).
  • totals.updates — lifetime revision count, excluding the initial save.
  • timeline post-update kind — revisions are collapsed per parent post (GROUP BY r.post_parent, MAX(post_date_gmt)) so a burst of saves on one article reads as a single row rather than flooding the timeline. Displayed with a pencil icon and "Updated "…"" label.
  • timeline[].type — CPT slug ('post', 'page', custom type) added to post and post-update rows so the renderer can pick a Post-vs-Page icon without a second REST lookup.
  • Streak logic — "active day" now includes update and comment activity, not only publish days.
  • UI "Updates" stat card — shown in the My WordPress stats panel when the lifetime count is > 0. Heatmap intensity now sums posts + comments + updates.

Tests

  • tests/vitest/dock-peek-native-icon-spawn.test.ts — verifies that a windowId-only dock item spawns a native window and does not fall through to the iframe path.
  • tests/vitest/my-wordpress-multi-instance.test.ts — covers the three multi-instance regressions above: close isolation, independent navigation, and unique drop-target ids.
Open WordPress Playground Preview

… issues

feat(my-wordpress): support multiple instances and ensure correct state handling
fix(ui): optimize canvas context for frequent read operations
test: add regression tests for dock-peek and multi-instance My WordPress behavior
@AllTerrainDeveloper

Copy link
Copy Markdown
Collaborator Author

@claude this is a PR that mostly fix small bugs, and also adds updates to the activity footprint. Can you review it?

@claude

claude Bot commented May 20, 2026

Copy link
Copy Markdown

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


PR Review: Pre Demo Polishment

  • Gather context and understand the request
  • Read changed files and diff
  • Review includes/my-wordpress/user-footprint.php
  • Review src/dock.ts
  • Review src/my-wordpress/index.ts + types.ts
  • Review src/ui/components/wpd-crumb-chain/wpd-crumb-chain.ts
  • Review src/ui/util/avatar-resolve.ts
  • Review new test files
  • Write and post full review

Summary

This is a solid polish PR. The three independent bug fixes are well-scoped and each has a clear reproduction story. The user-footprint feature extension is coherent and well-integrated. Tests are targeted and cover the exact regression scenarios described. A few issues worth flagging below.


1. Backward-compatibility gap in UserFootprint type — updates is required but old API won't send it

src/my-wordpress/types.ts line 276

updates is typed as number (required), but any site running an older version of the PHP (pre-0.8.7) will return a daily array without this key. The JS handles the missing value gracefully everywhere with ?? 0, but TypeScript won't warn you at the call sites that feed data from API responses — those go through as UserFootprint casts, so the required type contracts holds at compile time but breaks at runtime against an old server.

Two options:

  • Make it optional: updates?: number — then the ?? 0 fallbacks become required by the type checker, which is the safer pattern.
  • Keep it required but note the 0.8.7 minimum API version assumption prominently.

Fix this →


2. 'type' field on post-update timeline rows is undocumented

includes/my-wordpress/user-footprint.php line 393–399

post-update entries include a 'type' => $u['post_type'] field (the post's CPT slug), but:

  • The JSDoc header comment at the top of the PHP file (timeline: [ { kind, date, title, link, status, postId? } ]) doesn't list type.
  • The UserFootprint['timeline'] array member in types.ts also doesn't include it.
  • The client-side buildFootprintTimeline doesn't use it.

Is the type field intentional for future use (e.g. showing a "Page" vs "Post" icon in the timeline)? If so, add it to the typedef and the JSDoc header. If not, drop it. As-is it leaks into the REST response but nothing consumes it.

Fix this →


3. Three separate revision queries — tolerable, but the totals query is unbounded

includes/my-wordpress/user-footprint.php

The same r.post_author = %d / revision / inherit / post_date_gmt > parent join pattern runs three times:

  • $update_rows (per-day, date-bounded)
  • $timeline_updates (LIMIT 30, not date-bounded — intentional)
  • $totals_updates (lifetime, no date bound, no LIMIT)

The $totals_updates query on a high-volume site (hundreds of thousands of revisions) will do a full index scan every page load of My WordPress. This is the same concern that already exists for $totals_posts and $totals_comments, so it's not a new problem introduced here — just worth flagging so it's on the radar if performance becomes a concern.


4. Minor: dock.ts inline type cast is a bit brittle

src/dock.ts lines 1815–1820

const api = ( window as unknown as {
    wp?: { desktop?: { openNewWindow?: ( id: string, opts?: { source?: string } ) => boolean } };
} ).wp?.desktop;

The wp.desktop public API type is already defined elsewhere in the codebase (JavaScript reference, wp.desktop global surface). If that type is exported, importing it here avoids drift. Not a blocker, but if openNewWindow's signature ever changes, this inline cast won't catch it.


What looks good

  • Multi-instance fix (my-wordpress/index.ts): The switch from a global desktop-mode-window-closed listener to a returned per-instance teardown is the right architectural direction. The liveStates map cleanly replaces the singleton + sanity-check pattern, and the rejectIdCounter prevents the silent drop-target clobber. The code comments explain the "why" well.

  • Dock-peek fix (dock.ts): The item.windowId && !item.url guard is tight and handles the exact failure mode. The fallback (openNewWindow returns falsy → falls through to existing logic) makes it safe to deploy.

  • willReadFrequently: true (wpd-crumb-chain.ts, avatar-resolve.ts): Correct fix for the Chrome perf warning; the comments explain why it fires in these specific call patterns.

  • Streak logic (user-footprint.php): Including comments and updates in the "active day" definition for the streak counter is the right call for a CMS. The $is_active closure is clean.

  • Test coverage: Both test files are well-structured. The multi-instance test's acknowledgment that module-level side effects only run once (and preserving desktopModeNativeWindows between tests) avoids a subtle test ordering trap. The manager integration test at line 177 is the most valuable — it exercises the real close/teardown/reload path.


@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.


🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

@AllTerrainDeveloper

Copy link
Copy Markdown
Collaborator Author

@claude I'm testing you, can you even fill the PR description based on the changes in the commits?

@claude

claude Bot commented May 20, 2026

Copy link
Copy Markdown

Claude finished @AllTerrainDeveloper's task in 1m 38s —— View job


PR Description (generated from commits)

Here's a PR description you can paste into the PR body:


Pre-Demo Polishment

A set of targeted bug fixes and a feature extension to the user activity footprint. No architectural changes; each fix is self-contained and regression-tested.


Fixes

Dock-peek: native-window icons now open correctly (src/dock.ts)

Dock items whose target is a native window (My WordPress, Jorvy, plugin-registered launchers) carry a windowId but no url. Clicking them in dock-peek was falling through to the generic URL-based iframe path, which opened a chrome-only window with src="" — the user saw a title bar with no content.

The fix adds an early guard: if item.windowId && !item.url, the click is routed directly through wp.desktop.openNewWindow(), spawning a proper native instance. If openNewWindow returns falsy (old API, feature flag off) the existing fallback logic handles it transparently.

My WordPress: multiple instances no longer interfere with each other (src/my-wordpress/index.ts)

Three related issues when two My WordPress windows were open at the same time:

  1. Close clobbered live siblings. The previous teardown used a desktop-mode-window-closed document listener matched on the base WINDOW_ID. Every sibling's close event fired the same handler — closing instance B cleared instance A's state and left it broken. Fixed by having renderInto return a per-instance teardown that the framework calls only for that specific window.

  2. Navigation in a non-foreground instance threw. asRenderState compared against the singleton activeState. Drilling into a posts list inside a backgrounded duplicate triggered "host body does not match active state". Fixed by introducing a liveStates: Map<HTMLElement, RenderState> that lets asRenderState resolve the correct state for any live instance.

  3. Drag-rejection drop targets overwrote each other. Both instances registered a claimant with the same id (my-wordpress-reject), so the second registration silently replaced the first's element binding. Fixed by appending a per-mount monotonic counter (rejectIdCounter) to the id.

UI: willReadFrequently: true on pixel-readback canvas contexts (src/ui/components/wpd-crumb-chain/wpd-crumb-chain.ts, src/ui/util/avatar-resolve.ts)

Both wpd-crumb-chain (foreground-color picker) and avatar-resolve (transparent-pixel probe) create a 1×1 canvas and immediately call getImageData. Without the willReadFrequently hint Chrome backs the canvas with a GPU texture, so each getImageData triggers a GPU→CPU sync. The breadcrumb chain re-paints on every navigation; the avatar resolver fires once per unique URL across the page — often enough that Chrome was surfacing the "willReadFrequently" performance warning in both cases.


Feature: User activity footprint now tracks post updates / revisions

includes/my-wordpress/user-footprint.php · src/my-wordpress/index.ts · src/my-wordpress/types.ts

The activity heatmap and stats previously only counted published posts and comments. Editors who spend most of their time updating existing content (the typical CMS workflow) appeared to have no activity. This brings the footprint model in line with how GitHub counts contributions — edits to existing content are first-class activity.

What counts as an "update": a WordPress revision saved by the user after the parent post's initial creation (r.post_date_gmt > p.post_date_gmt). The initial save is excluded so it doesn't double-count with the "posts" column. Updates are attributed to whoever hit Save (via r.post_author), not the post owner — consistent with tracking editorial work across a team.

Changes in detail:

  • daily[].updates — per-day update count, added alongside the existing posts and comments fields (zero-filled for days with no saves).
  • totals.updates — lifetime revision count, excluding the initial save.
  • timeline post-update kind — revisions are collapsed per parent post (GROUP BY r.post_parent, MAX(post_date_gmt)) so a burst of saves on one article reads as a single row rather than flooding the timeline. Displayed with a pencil icon and "Updated "…"" label.
  • timeline[].type — CPT slug ('post', 'page', custom type) added to post and post-update rows so the renderer can pick a Post-vs-Page icon without a second REST lookup.
  • Streak logic — "active day" now includes update and comment activity, not only publish days.
  • UI "Updates" stat card — shown in the My WordPress stats panel when the lifetime count is > 0. Heatmap intensity now sums posts + comments + updates.

Tests

  • tests/vitest/dock-peek-native-icon-spawn.test.ts — verifies that a windowId-only dock item spawns a native window and does not fall through to the iframe path.
  • tests/vitest/my-wordpress-multi-instance.test.ts — covers the three multi-instance regressions above: close isolation, independent navigation, and unique drop-target ids.

@AllTerrainDeveloper AllTerrainDeveloper merged commit 1960fc2 into trunk May 20, 2026
5 checks passed
@AllTerrainDeveloper AllTerrainDeveloper deleted the pre-demo-polishment branch May 20, 2026 12:31
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