CmdPal JS/TS Extensions - Phase 7: details size, prefix-select, image fix (+ design proposals) - #49364
Open
michaeljolley wants to merge 17 commits into
Conversation
Details size: - ts-sdk: add optional DetailsSize (small/medium/large or numeric ContentSize) to the Details type and serialize it. - JSModelMapper.ParseDetails: map the details "size" field (string or number) onto the toolkit Details.Size, which already flows to the host via IExtendedAttributesProvider.GetProperties()["Size"]. - Sample details page asks for a large details pane on the metadata item. - Tests: C# adapter mapping (string + number + default) and ts-sdk serialize. Image content sample: - The image content page used a remote https URL, which does not render in the host (the same reason the hero image was localized in Phase 6). Point it at the committed local hero.png asset instead. Prefix-select (textToSuggest): - No code change needed: the TS SDK, wire serializer, and C# JSListItemAdapter already carry textToSuggest end to end, and the sample sets it. Selection fill is gated purely by the host env flag CMDPAL_ENABLE_SUGGESTION_SELECTION. Added ts-sdk serialize tests to lock the textToSuggest wire field. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 55931a4e-bdcf-40f4-8a60-06ad848cea59
Per the orchestrator go/no-go, per-page OnLoad/OnUnload and drag-drop (DataPackage) are deferred, not implemented. Add a "Known Gaps and Deferred Work" section to the JS extension spec overview describing what is missing, why it is deferred (no C# ABI parity for page lifecycle; no DataPackage in the ABI), and the likely shape of a future solution for each. Docs only. No code, XAML, or wire surface changed. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 55931a4e-bdcf-40f4-8a60-06ad848cea59
Addresses a dogfooding punch list for the JS/TS sample extension. Each item was root-caused and fixed in the correct layer. No new protocol or wire surface was added. 1. grid-images: The section sample items carried no icon, so grid and gallery variants showed empty tiles. The C# SampleListPageWithSections gives every item a bundled image. Fixed in the sample by assigning the committed local hero.png asset to each section item, resolved from import.meta.url. 2. section-separator-line: A titled Separator renders as heading text with no divider line; a plain Separator renders a horizontal line. This host behavior is shared with the built-in C# section pages, so the C# built-ins show no line either. Fixed in the sample by emitting a plain Separator divider line between sections in addition to the titled heading. 3. details-buttons-noop: Traced end to end and confirmed the JS DetailsCommands to command/invoke path is fully wired; clicking a details command button sends command/invoke and the extension responds. Verified over stdio. Added a unit test that maps a details commands element and asserts invoking it sends command/invoke with the correct command id. 4. onunload-demo: OnUnload has no protocol support. Reworded the OnLoad sample and its registration to demonstrate OnLoad only, dropping all unload wording. 5. prefix-select: The type-ahead selection rides a pre-existing host feature flag that ships off by default, so the demo did nothing for users. Removed the prefixed-search sample page and its registration. The textToSuggest SDK and adapter plumbing is correct and stays in place. 6. dragdrop-sample-remove: Drag and drop is a documented gap. Removed the drag-and-drop bits from the clipboard sample, leaving a clipboard-only demo. The documented gap note in the spec is left intact. Also removed two now-stale README lines: the prefix suggestions bullet and the details-size not-mirrored entry that Phase 7 already delivered. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 55931a4e-bdcf-40f4-8a60-06ad848cea59
This was referenced Jul 21, 2026
michaeljolley
marked this pull request as ready for review
July 21, 2026 17:04
Co-authored-by: Copilot App <[email protected]> Copilot-Session: 762976e0-2453-40ca-8034-978a04a19d1e
…ditions) Co-authored-by: Copilot App <[email protected]> Copilot-Session: 3f0875f9-177f-40db-888c-248a044d2bb9
…tions Co-authored-by: Copilot App <[email protected]> Copilot-Session: 9470f549-1997-44de-b0a6-1ce4562a51f0
… phase-7-cmdpal-protocol-additions
…l-protocol-additions Propagate the phase-5 fail-closed npm install transaction (carried through the phase-6 docs and sample merge) into the protocol-additions phase. The merge was clean with no conflicts. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 234c59f1-a32b-4c98-97bb-ab50a4d07fd9
Propagates the stale-test and golden-fixture corrections so the SDK typecheck stays green at this layer, including phase-7 protocol tests. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
Propagates removal of the unused sent binding so npm run check (lint included) stays green at this layer. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
…ter tests Corrects the drag-and-drop (DataPackage) subsection of the JS extension overview to state accurately that the C# extension ABI already models a draggable payload via IExtendedAttributesProvider exposing WellKnownExtensionAttributes.DataPackage (surfaced as CommandItemViewModel.DataPackage), and that the host list already acts as a drag source that consumes it. Frames the remaining deferred work as JS-only: an SDK surface to declare the payload, JSON-RPC transport, and adapter materialization into the existing DataPackage attribute. No new WinRT ABI or WinUI drag source or drop target work is required. Adds additive tests around already-correct production mappings: - TextToSuggest maps through JSListItemAdapter for present, omitted, and empty-string wire values (defaults to string.Empty). - Image content preserves a wire image path verbatim into the materialized ImageContent.Image.Light.Icon; containment lives at the manifest-icon layer. - Details size falls back to Small for malformed, unknown-string, and future-numeric wire values. - TS serializer emits an empty textToSuggest and (already covered) omits size when unset. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
Bring the phase-6 docs/sample round-2 fixes down onto phase-7. Clean merge. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 311db8aa-f8db-47bb-943f-5447037946c7
michaeljolley
force-pushed
the
dev/mjolley/phase-6-cmdpal-docs-sample
branch
from
July 27, 2026 21:14
efdf0b5 to
d76e2b0
Compare
michaeljolley
force-pushed
the
dev/mjolley/phase-7-cmdpal-protocol-additions
branch
from
July 27, 2026 21:15
ab0ebfe to
33621d9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This PR is the last in a series focused on adding TypeScript/JavaScript extension support to Command Palette. A detailed design can be located in the initial PR #49321.
Phase 7: CmdPal JS/TS Extensions - protocol additions
Stacked on
dev/mjolley/phase-6-cmdpal-docs-sample(this PR targets that branch, notmain). Part of the JS/TS Command Palette extensions effort tracked by #48707.This phase delivers three items that are fully feasible inside the existing C# ABI (no new IDL, no new wire methods), plus two design proposals that need a go/no-go before any new wire surface is added.
1. Details size (small / medium / large)
The host already supports sized details panes:
DetailsViewModelreadsIExtendedAttributesProvider.GetProperties()["Size"]as an int and casts it toContentSize(Small=0, Medium=1, Large=2), andDetailsSizeToGridLengthConverterconsumes it. The toolkitDetailstype already implementsIExtendedAttributesProviderand returns{ "Size": (int)Size }. The only gaps were in the JS layers.Changes:
types.ts,serialize.ts): added an optionalsizefield toDetails, typedDetailsSize | numberwhereDetailsSize = 'small' | 'medium' | 'large', and serialize it on the wire (omitted when unset).JSModelMapper.ParseDetails): map the detailssizefield ontoDetails.Size. A newParseContentSizehelper accepts either the numeric ContentSize (0/1/2) or the string form ('small'/'medium'/'large') and defaults toSmall.detailsPage.ts): the metadata details item now requestssize: 'large'so the larger pane is visible.Wire shape (Details):
"size": "large"or"size": 2. Both map toContentSize.Large.2. Prefix-select (textToSuggest fills the search box)
Root-caused as already wired end to end. The TS SDK (
IListItem.textToSuggest), the wire serializer (serialize.ts), and the C#JSListItemAdapterall carrytextToSuggest, and the hostListViewModelpostsUpdateSuggestionMessageon selection.The only reason selection does not fill the box by default is the host feature flag
IsTextToSuggestEnabledinSearchBar.xaml.cs, which reads the environment variableCMDPAL_ENABLE_SUGGESTION_SELECTIONand is off by default. That flag ships onmain(authored by a maintainer, not introduced by these phases) and is intentionally NOT force-enabled here. ThetextToSuggestSDK and adapter plumbing is correct and stays in place; a ts-sdk serialize test locks the wire field so a future refactor cannot silently drop it. See Round 2 item 5 for the fate of the sample demo page.3. Image content sample renders nothing
The Phase 4 crash fix (short-circuiting
IconLoaderServicescaled size toSize.EmptywheniconSize.IsEmpty) is not the cause. That no-resize path still loads string and URL icons; the details hero image flows through the same path and renders correctly because Phase 6 localized it to a packaged asset.Root cause: the image content sample page still pointed at a remote https URL, which does not render in the host. Only the hero (details) image was localized in Phase 6.
Change (
contentPages.ts):SampleImageContentPagenow references the committed localassets/hero.pngvia thefileURLToPath(new URL(...))pattern already used by the details page. No host code path changed, so no host tests were added.Validation (Round 1)
npm run buildclean, tests pass, eslint clean.npm run buildexit 0.CommandPalette.slnf, x64 Debug): exit 0.*.UnitTestsassemblies green.No XAML was changed. No warnings suppressed. No emdash used.
Deferred design items (documented as known gaps)
Per the orchestrating workstream's go/no-go, Phase 7 does not implement the two design proposals below. They are documented as known gaps in
doc/json-rpc-spec/overview.mdand deferred to a future phase. No new wire surface was added.4. Per-page lifecycle: OnLoad / OnUnload (deferred)
IPage(IDL) has no OnLoad/OnUnload. In the JS runtime, "OnLoad" already happens implicitly: the firstgetItemsfetch after navigation is the de facto load signal. There is no equivalent for a page being navigated away from, and no C# ABI notification exists for either direction. The likely future shape is an additive JS-onlypage/unloadednotification emitted atPageViewModel.UnsafeCleanup(), with no C# parity and no WinRT IDL change.5. Drag and drop (DataPackage) (deferred)
There is no drag-drop or DataPackage concept anywhere in the ABI, C# or JS. The only related primitive is clipboard. The minimal future path is copy-on-drag built on the existing clipboard primitive; the fuller path is a dedicated DataPackage-style wire payload plus host drag source and drop target handling, which is the only shape that would touch the WinRT ABI.
Round 2: dogfooding sample fixes
A follow-up dogfooding punch list against the sample extension. Every item was root-caused and fixed in the correct layer. No new protocol or wire surface was added.
Grid images missing (sample layer). Section sample items carried no icon, so grid and gallery variants showed empty tiles, while the C#
SampleListPageWithSectionsgives every item a bundled image. Fixed by assigning the committed localhero.pngasset to each section item (sectionsPages.ts), resolved fromimport.meta.url.Section separator line missing (sample layer, host-vs-sample surprise). A titled
Separatorrenders as heading text with no divider line; a plainSeparatorrenders a horizontal line. This host behavior is shared with the built-in C# section pages, so the C# built-ins show no line under a titled heading either. Rather than change the shared host template (which would alter the CmdPal home page and every C# extension), the sample now emits a plainSeparatordivider line between sections in addition to the titled heading.Details buttons do nothing (no defect found). Traced end to end and confirmed the JS
DetailsCommandstocommand/invokepath is fully wired; clicking a details command button sendscommand/invokeand the extension responds with a status message. Verified over stdio. Added a unit test (JSAdapterProxyTests) that maps a details commands element and asserts invoking it sendscommand/invokewith the correct command id. The earlier "noop" report did not reproduce.OnUnload demo (sample layer). OnUnload has no protocol support (see deferred item 4). The OnLoad sample and its registration now demonstrate OnLoad only, dropping all unload wording so the sample never implies unsupported behavior.
Remove prefixed-search demo (sample layer). Because the type-ahead selection rides the pre-existing host feature flag that ships off by default, the demo did nothing for users. Removed the prefixed-search sample page and its registration. The
textToSuggestSDK and adapter plumbing (Round 1 item 2) is legitimate and stays in place.Remove drag-and-drop sample (sample layer). Drag and drop is a documented gap (deferred item 5). Removed the drag-and-drop bits from the clipboard sample, leaving a clipboard-only demo. The documented gap note in the spec is left intact.
Also removed two now-stale README lines: the prefix suggestions bullet and the details-size "not mirrored" entry that Phase 7 already delivered.
Validation (Round 2)
npm run buildexit 0. ts-sdk untouched this round.CommandPalette.slnf, x64 Debug): exit 0.*.UnitTestsvia the built x64 MTP DLLs: all green.Microsoft.CmdPal.UI.ViewModels.UnitTests233/233 (including the new details-command invoke test), Toolkit 646, Common 225, Bookmarks 222, TimeDate 301, and the rest passing;WindowWalkerreports "zero tests ran" (known harness quirk).No XAML was changed. No warnings suppressed. No emdash used.
Draft. The three Round 1 items are complete and validated, the two design items are documented as known gaps, and the Round 2 dogfooding punch list is addressed entirely in the sample layer plus one host-mapping unit test.