diff --git a/apps/desktop/src/components/SessionRenameDialog.tsx b/apps/desktop/src/components/SessionRenameDialog.tsx index 5e4de76d33..89c3c0e899 100644 --- a/apps/desktop/src/components/SessionRenameDialog.tsx +++ b/apps/desktop/src/components/SessionRenameDialog.tsx @@ -1,4 +1,5 @@ import { useEffect, useRef, useState, type FormEvent } from "react"; +import { useBlockingOverlay } from "../lib/blocking-overlay"; import { portalToBody } from "../lib/portal-visibility"; import { useTranslation } from "react-i18next"; import { MAX_SESSION_TITLE_LENGTH } from "@pi-desktop/shared"; @@ -40,6 +41,10 @@ function RenameDialog({ onSave, onError, }: RenameDialogProps) { + // Electron's native preview is composited above renderer DOM, including + // portals and the top layer. Hide it for the lifetime of this modal so the + // rename surface remains fully visible and clickable in three-column mode. + useBlockingOverlay(); const [draft, setDraft] = useState(value); const [saving, setSaving] = useState(false); const savingRef = useRef(false); diff --git a/apps/desktop/src/components/settings/ModelSelectionPanes.tsx b/apps/desktop/src/components/settings/ModelSelectionPanes.tsx index 3828c2fa52..f755eb7664 100644 --- a/apps/desktop/src/components/settings/ModelSelectionPanes.tsx +++ b/apps/desktop/src/components/settings/ModelSelectionPanes.tsx @@ -233,9 +233,10 @@ export function ModelSelectionPanes({ const [chosenQuery, setChosenQuery] = useState(""); const [customModelId, setCustomModelId] = useState(""); const [customModelError, setCustomModelError] = useState(""); - const [expandedModelId, setExpandedModelId] = useState( - () => models[0]?.id ?? null, - ); + // Keep fetched selections scannable. Expanding the first row by default can + // fill the pane with its controls and push every other checked model below + // the fold, which makes a successful multi-select look empty. + const [expandedModelId, setExpandedModelId] = useState(null); // The returned list is short and already local, so filtering is client-side: // no host search and no debounced IPC round trip. @@ -313,7 +314,6 @@ export function ModelSelectionPanes({ (binding) => binding.id.toLowerCase() === wanted, ); if (!alreadyChosen) { - setExpandedModelId((open) => open ?? row.id); keepAddedModelVisible([bindingForRow(row)]); } setModels((current) => { @@ -326,7 +326,6 @@ export function ModelSelectionPanes({ const toggleVisibleModels = (select: boolean) => { if (select) { - setExpandedModelId((open) => open ?? visibleRows[0]?.id ?? null); const added = visibleRows .filter((row) => !selected.has(row.id.toLowerCase())) .map((row) => bindingForRow(row)); diff --git a/apps/desktop/test/fixtures/dialog-overflow-runner.cjs b/apps/desktop/test/fixtures/dialog-overflow-runner.cjs index 062548c563..15f7f96127 100644 --- a/apps/desktop/test/fixtures/dialog-overflow-runner.cjs +++ b/apps/desktop/test/fixtures/dialog-overflow-runner.cjs @@ -68,12 +68,40 @@ app.whenReady().then(async () => { await evaluate('window.dialogFixture.show(' + JSON.stringify(kind) + ')'); const m = await measure(); check('audit ' + kind + ' long text stays bounded', m.contained && m.overflow <= 1 && m.closeContained && m.closeHit, m); + if (kind === 'rename') { + check('rename suppresses native preview while open', await evaluate('window.dialogFixture.isBlockingOverlayActive()')); + } const dismiss = kind === 'rename' ? '.session-rename-dialog-close' : ['instructions','memory','delete'].includes(kind) ? '.project-instructions-dialog-close' : kind === 'oauth' ? '.provider-dialog-actions button' : '.plugins-modal-actions button'; await click(dismiss); check('audit ' + kind + ' dismissal remains operable', await evaluate('window.dialogFixture.closed && !document.querySelector("[role=dialog]")')); + if (kind === 'rename') { + check('rename releases native preview after close', await evaluate('!window.dialogFixture.isBlockingOverlayActive()')); + } } + win.setContentSize(1100, 620); + await evaluate('window.dialogFixture.show("models", { theme: "dark", locale: "zh-CN" })'); + const modelRows = await evaluate(`(() => { + const list = document.querySelector('.provider-chosen-list'); + const bounds = list.getBoundingClientRect(); + const rows = [...list.querySelectorAll('.provider-chosen-row')]; + return { + ids: rows.map(row => row.querySelector('.provider-chosen-row-id')?.textContent), + expanded: rows.filter(row => !row.querySelector('.provider-chosen-row-body')?.hidden).length, + allVisible: rows.every(row => { + const rect = row.querySelector('.provider-chosen-row-head').getBoundingClientRect(); + return rect.top >= bounds.top - 1 && rect.bottom <= bounds.bottom + 1; + }), + }; + })()`); + check('chosen model names are visible before Advanced opens', + modelRows.ids.length === 4 && modelRows.ids.every(Boolean) && modelRows.expanded === 0 && modelRows.allVisible, + modelRows); + writeFileSync(join(process.env.PI_DIALOG_ARTIFACT_DIR, 'chosen-models-collapsed.png'), (await win.webContents.capturePage()).toPNG()); + await click('.provider-chosen-advanced-toggle'); + check('Advanced still expands one selected model on demand', + await evaluate(`document.querySelectorAll('.provider-chosen-row-body:not([hidden])').length === 1 && document.querySelector('.provider-chosen-advanced-toggle').getAttribute('aria-expanded') === 'true'`)); writeFileSync(join(process.env.PI_DIALOG_ARTIFACT_DIR, 'results.json'), JSON.stringify(results, null, 2)); const failed = results.filter(r => !r.ok).length; console.log('SUMMARY ' + (results.length-failed) + '/' + results.length + ' passed'); diff --git a/apps/desktop/test/fixtures/dialog-overflow.jsx b/apps/desktop/test/fixtures/dialog-overflow.jsx index f55fffe32d..e057503d86 100644 --- a/apps/desktop/test/fixtures/dialog-overflow.jsx +++ b/apps/desktop/test/fixtures/dialog-overflow.jsx @@ -1,10 +1,10 @@ -import React from "react"; +import React, { useState } from "react"; import { createRoot } from "react-dom/client"; import { flushSync } from "react-dom"; import i18n from "i18next"; import { initReactI18next } from "react-i18next"; import { catalogs, flattenCatalog } from "@pi-desktop/i18n"; -import { IPC } from "@pi-desktop/shared"; +import { bindingForCustomModel, IPC } from "@pi-desktop/shared"; import { ExtensionPromptHost } from "../../src/components/ExtensionPromptDialog"; import { SessionRenameDialog } from "../../src/components/SessionRenameDialog"; import { ProjectInstructionsDialog } from "../../src/components/ProjectInstructionsDialog"; @@ -15,6 +15,8 @@ import { OAuthLoginDialog } from "../../src/components/settings/OAuthLoginDialog import { PluginDialogs } from "../../src/features/plugins/PluginDialogs"; import { PluginSettingsSheet } from "../../src/components/plugins/PluginSettingsSheet"; import { newInstallJob } from "../../src/features/plugins/install-progress"; +import { ModelSelectionPanes } from "../../src/components/settings/ModelSelectionPanes"; +import { isBlockingOverlayActive } from "../../src/lib/blocking-overlay"; const listeners = new Map(); const path = "C:\\Users\\Example\\AppData\\Local\\Temp\\pi-extension-fixture\\plugins\\greet\\src\\greet.ts"; @@ -40,8 +42,36 @@ let revision = 0; const frame = () => new Promise(requestAnimationFrame); const close = () => { window.dialogFixture.closed = true; flushSync(() => root.render(null)); }; const error = (value) => { throw value; }; + +const modelIds = ["grok-4.5", "grok-4.6", "grok-4.7", "grok-4.7-build-fast"]; +function ModelPickerFixture() { + const [models, setModelsState] = useState(() => modelIds.map(bindingForCustomModel)); + const rows = modelIds.map((id) => ({ + id, + displayName: id.replace("grok", "Grok"), + contextWindow: 200_000, + maxTokens: 4_000, + })); + return ( +
+ setModelsState((current) => update(current)), + }} + listTitle="Service models" + /> +
+ ); +} + window.dialogFixture = { responses: [], closed: false, saved: null, + isBlockingOverlayActive, async show(kind, options = {}) { flushSync(() => root.render(null)); this.responses = []; this.closed = false; this.saved = null; @@ -62,6 +92,7 @@ window.dialogFixture = { subscribe(listener) { queueMicrotask(() => listener({ kind: "authUrl", url: "https://example.invalid/" + long, instructions: "Open the sign-in URL", opened: false })); return () => {}; }, cancel: async () => {}, }} />; + if (kind === "models") component = ; flushSync(() => root.render(component)); await frame(); if (kind === "extension") { diff --git a/apps/desktop/test/model-advanced-capabilities.test.mjs b/apps/desktop/test/model-advanced-capabilities.test.mjs index ba386e2520..8808ef4fda 100644 --- a/apps/desktop/test/model-advanced-capabilities.test.mjs +++ b/apps/desktop/test/model-advanced-capabilities.test.mjs @@ -236,7 +236,8 @@ test("the advanced body is a compact sheet without helper paragraphs", () => { ); assert.doesNotMatch(pickerSource, /hint=\{t\("settings\.modelAliasHint"\)\}/); assert.match(pickerSource, /aria-controls=\{advancedId\}/); - assert.match(pickerSource, /models\[0\]\?\.id \?\? null/); + // Keep the selected-model summary visible until Advanced is requested. + assert.match(pickerSource, /useState\(null\)/); assert.match( pickerSource, /className="provider-chosen-thinking-head">[\s\S]*?provider-chosen-thinking-default[\s\S]*?provider-chosen-thinking-chips/, diff --git a/apps/desktop/test/provider-model-config.test.mjs b/apps/desktop/test/provider-model-config.test.mjs index 8aae8f2b49..cc3ce1beb7 100644 --- a/apps/desktop/test/provider-model-config.test.mjs +++ b/apps/desktop/test/provider-model-config.test.mjs @@ -174,6 +174,18 @@ test("the shared picker owns the advanced per-model controls for both kinds", () assert.match(pickerSource, /bindingsToPersist/); }); +test("fetched model selections stay collapsed until Advanced is requested", () => { + assert.match( + pickerSource, + /const \[expandedModelId, setExpandedModelId\] = useState\(null\)/, + ); + assert.doesNotMatch( + pickerSource, + /setExpandedModelId\(\(open\) => open \?\? (?:row\.id|visibleRows\[0\])/, + ); + assert.match(pickerSource, /current === binding\.id \? null : binding\.id/); +}); + test("a vendor account saves explicit bindings, not raw state", () => { // The shared picker preserves explicit thinking levels, including a manual // override not present in the catalog. diff --git a/apps/desktop/test/session-rename.test.mjs b/apps/desktop/test/session-rename.test.mjs index c3e8a3a00b..7490695988 100644 --- a/apps/desktop/test/session-rename.test.mjs +++ b/apps/desktop/test/session-rename.test.mjs @@ -49,6 +49,7 @@ test("rename dialog is modal, localized, and caps input by Unicode code points", assert.match(dialogSource, /Array\.from\(event\.target\.value\)/); assert.match(dialogSource, /MAX_SESSION_TITLE_LENGTH/); assert.match(dialogSource, /t\("session\.renameSave"\)/); + assert.match(dialogSource, /useBlockingOverlay\(\)/); assert.match(styles, /\.session-rename-dialog-overlay\s*\{[^}]*z-index:\s*65;/s); }); diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index 3b35677a62..0bcf9f56cd 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -582,7 +582,7 @@ identify the platform validation still needed. - **Preconditions**: App running; no provider configured; the models.dev snapshot ships with the build. - **Steps**: 1) Open Settings → Model configuration and choose Add provider. 2) Confirm the dialog is ONE form with no stepper or Next/Back buttons. The first control is Service — a searchable menu (Choose a service, Custom endpoint, then a flat vendor list from models.dev including Xiaomi), not a native select, region grouping, or vendor-card grid. Open it, type to filter client-side, then choose **Custom endpoint**. Confirm Name and Base URL appear on one row with no helper paragraph under the URL (placeholder only), API Key and API format appear side by side on the next row (not behind Advanced), and that a focused field plus its 2px accent ring stays fully inside the dialog, including on a window narrower than 1040px. 3) Enter a name and a base URL for a service that publishes a `/models` route, then paste an API key. 4) Confirm the models section fills with the models THAT SERVICE returned, not with every model its vendor publishes; confirm a model the deployment does not host is absent. 5) Type in the filter box and confirm the list narrows client-side with no network request per keystroke. 6) Confirm each row shows the models.dev-derived context/output for models the catalog knows, that its compact text tracks the published value instead of a coarser rounded one (a 1,050,000 window reads `1.05M`, never `1.1M`), and that a model with no catalog match still lists with generic defaults. 7) Select two models with the checkboxes. 8) Expand Advanced on one chosen row, override its limits and toggle thinking chips; confirm each numeric field has a five-chip preset ladder for common values, clicking a chip writes the value, hand editing remains possible, and a non-preset value leaves the ladder unselected. Confirm the label and optional hint sit above one compact grouped control and do not force the options onto a second row at normal dialog width; confirm all seven canonical levels are available, that published levels start selected for a known reasoning model, and that a non-reasoning or unknown row shows the same chips unselected with the manual-override hint; enable one level on that row and confirm the other row is unaffected. 9) Open the form-level Advanced and confirm the API format is present but pre-derived. 10) Add a free-form model ID the service did not return; confirm it is added with 128,000 / 8,192 / no-thinking defaults, then enable a thinking level if the endpoint supports it; confirm re-adding the same ID in different letter case is rejected as already added. 11) Save. -- **Expected**: The service is asked first and models.dev only enriches the answer and seeds known-model defaults. The settings picker always offers the seven canonical thinking levels, and the Composer later renders the explicit levels saved in the same model binding; an empty or `off`-only binding resolves to `off`. Discovery is debounced ~600 ms, does not mark loading until that window elapses, and a slow reply from an earlier keystroke never replaces a newer list; named add-path discovery waits for an API key, while an unsaved custom provider is probed with the typed base URL (and key, if any) before it exists. Preset ladders cover common context/output limits while preserving hand-edited values. Limit text renders through one shared compact formatter, so neighbouring published windows stay distinguishable (`1M` / `1.05M` / `1.1M`) and a compact limit never reads above its published value. Custom endpoint keeps API format beside the key and omits Base URL helper copy; named endpoints do not show format. Point the same custom form at an unreachable or unauthorized URL and confirm the left pane shows a classified error (not a raw JSON/HTML dump and not a second “no models” empty state); with cached rows from a later edit, the same error is a one-line banner above the list. Point a second provider at a base URL with no `/models` route and confirm the list falls back to the catalog, is labelled as coming from models.dev rather than the service, and still saves. The provider appears as a row with its host, model count and secret badge; the key is stored securely (not in plaintext config); `models` contains both bindings and `models[0]` remains the provider default. +- **Expected**: The service is asked first and models.dev only enriches the answer and seeds known-model defaults. Newly fetched or checked model rows stay collapsed until the user opens Advanced, so every selected model ID remains visible in the right pane after a multi-select. The settings picker always offers the seven canonical thinking levels, and the Composer later renders the explicit levels saved in the same model binding; an empty or `off`-only binding resolves to `off`. Discovery is debounced ~600 ms, does not mark loading until that window elapses, and a slow reply from an earlier keystroke never replaces a newer list; named add-path discovery waits for an API key, while an unsaved custom provider is probed with the typed base URL (and key, if any) before it exists. Preset ladders cover common context/output limits while preserving hand-edited values. Limit text renders through one shared compact formatter, so neighbouring published windows stay distinguishable (`1M` / `1.05M` / `1.1M`) and a compact limit never reads above its published value. Custom endpoint keeps API format beside the key and omits Base URL helper copy; named endpoints do not show format. Point the same custom form at an unreachable or unauthorized URL and confirm the left pane shows a classified error (not a raw JSON/HTML dump and not a second “no models” empty state); with cached rows from a later edit, the same error is a one-line banner above the list. Point a second provider at a base URL with no `/models` route and confirm the list falls back to the catalog, is labelled as coming from models.dev rather than the service, and still saves. The provider appears as a row with its host, model count and secret badge; the key is stored securely (not in plaintext config); `models` contains both bindings and `models[0]` remains the provider default. - **Specs linked**: `03-runtime/11-provider-model-system.md`, `03-runtime/12-provider-config-schema.md`, `03-runtime/13-model-catalog-and-selection.md`, `03-runtime/14-secrets-storage.md`, `04-ux/06-settings-ia.md` - **Acceptance**: B (multi-model provider configuration, save key) - **Milestone**: M2 @@ -3584,7 +3584,8 @@ identify the platform validation still needed. - **Steps**: 1) Activate the artifact, enter `localhost:` without a scheme, and submit. 2) Navigate site links; use back/forward/reload/stop. 3) Trigger a `window.open` popup and a permission-requesting page (e.g. notification - prompt). 4) Open global search, then Settings. Return to chat + prompt). 4) Open global search, then rename a session from the left sidebar; + close it and open Settings. Return to chat and trigger an inline tool permission card. 5) Switch to another panel tab and back; close the panel. 6) Use open-external. - **Expected**: Scheme-less input normalizes to http; nav state (URL bar, @@ -3592,7 +3593,8 @@ identify the platform validation still needed. the default browser (never in-app) only when the URL parses as http(s) or mailto; `file:`, `javascript:`, and custom schemes are denied. Permission requests are denied; non-http(s) navigation is blocked except in-root - `file:` siblings. The preview hides under every blocking overlay and while + `file:` siblings. The preview hides for the whole lifetime of the rename + dialog and every other blocking overlay, and while unmounted, reappearing with correct bounds afterwards. An inline permission card does not hide or remount the preview; resize/drag keeps the native view visible and aligned with the placeholder rect without a black flash. Opening