Put the provider/model picker above the dialog that opens it
The picker (node-settings-dialog) shared --z-modal with every other dialog guest, so opening it from "Evaluate impact with LLM" - itself a modal - tied on z-index with the report behind it and lost on DOM order: it rendered, and its backdrop even blocked clicks, but neither was visible. It read as a button that did nothing. Named the layer this actually is - --z-dialog-over-modal, the same one the confirmation dialog already needed and had defined ad hoc as --z-confirm - and moved the picker onto it. While chasing this, closed a real silence next to it: with no LLM provider published at all, the picker answered null and the caller treated that like a dismissal, so the button did nothing for a second, unrelated reason. It now throws with a message, and Simulate surfaces it as a notification instead of swallowing it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
68f4b6db85
commit
03ac4901ed
|
|
@ -72,10 +72,17 @@ describe('openLLMDescriptorSettings', () => {
|
|||
expect(open.mock.calls[0][0].initial.seed).toBeUndefined();
|
||||
});
|
||||
|
||||
it('answers null when no provider is published at all', async () => {
|
||||
it('says so, instead of opening nothing, when no provider is published at all', async () => {
|
||||
const { service, open } = dialog({ provider: 'x', model: 'y' });
|
||||
|
||||
expect(await openLLMDescriptorSettings(service, retriever([]), { title: 'Simulation Settings' })).toBeNull();
|
||||
await expect(openLLMDescriptorSettings(service, retriever([]), { title: 'Simulation Settings' }))
|
||||
.rejects.toThrow('No LLM provider is published');
|
||||
expect(open).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('answers null when the dialog is dismissed', async () => {
|
||||
const { service } = dialog(null);
|
||||
|
||||
expect(await openLLMDescriptorSettings(service, retriever(), { title: 'Simulation Settings' })).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -48,7 +48,8 @@ export type LLMDescriptorSettingsRequest = {
|
|||
* They ask the same question, so they ask it with the same window - the provider list, the models
|
||||
* that follow from the chosen provider, and the same optional parameters.
|
||||
*
|
||||
* Returns null when the dialog is dismissed, or when no provider is published at all.
|
||||
* Returns null when the dialog is dismissed. Throws when no provider is published at all: the
|
||||
* caller has a place to say so, and answering null there made the button that opened it look broken.
|
||||
*/
|
||||
export async function openLLMDescriptorSettings(
|
||||
settingsDialog: NodeSettingsDialogService,
|
||||
|
|
@ -57,7 +58,7 @@ export async function openLLMDescriptorSettings(
|
|||
): Promise<LLMDescriptor | null> {
|
||||
const providerOptions = await loadOptions(fieldRetriever, 'providers', {}, PROVIDER_RETRIEVER_URL);
|
||||
if (!providerOptions.length) {
|
||||
return null;
|
||||
throw new Error('No LLM provider is published by the server, so there is no model to choose.');
|
||||
}
|
||||
|
||||
const inherited = request.initialDescriptor ?? null;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
@if (state()) {
|
||||
<div class="fixed inset-0 z-(--z-modal)">
|
||||
<div class="fixed inset-0 z-(--z-dialog-over-modal)">
|
||||
<div class="absolute inset-0 bg-black/50" (click)="cancel($event)"></div>
|
||||
|
||||
<div class="absolute left-1/2 top-1/2 flex max-h-[min(80vh,720px)] w-[min(92vw,560px)] -translate-x-1/2 -translate-y-1/2 flex-col gap-4 overflow-hidden rounded-2xl border border-(--color-border) bg-white p-6 shadow-(--shadow-modal)">
|
||||
|
|
|
|||
|
|
@ -527,7 +527,9 @@
|
|||
[executionId]="execution()!.id"
|
||||
[annotatedNodeCount]="biasAnnotatedNodeCount()"
|
||||
[blockedReason]="biasExperimentBlockedReason()"
|
||||
(startExperimentRequested)="openBiasedRerunDialog()" />
|
||||
[comparableVariant]="canCompareBiasExecution()"
|
||||
(startExperimentRequested)="openBiasedRerunDialog()"
|
||||
(compareRequested)="openCompareDialog()" />
|
||||
}
|
||||
</div>
|
||||
</aside>
|
||||
|
|
|
|||
|
|
@ -1132,7 +1132,19 @@ export class TaskExecutionViewerComponent implements OnDestroy {
|
|||
const executionId = this.execution()?.id;
|
||||
if (!executionId || !this.canSimulateExecution()) return;
|
||||
|
||||
const simulator = await this.openSimulationSettings();
|
||||
let simulator: LLMDescriptor | null;
|
||||
try {
|
||||
simulator = await this.openSimulationSettings();
|
||||
} catch (error) {
|
||||
// No provider to choose from, or the list could not be loaded. Silence here reads as a dead
|
||||
// button, which is what the picker is opened by.
|
||||
this.notifications.show(
|
||||
error instanceof Error ? error.message : 'Unable to load the providers to simulate with.',
|
||||
'error',
|
||||
6000
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (!simulator) return;
|
||||
|
||||
this.simulateInProgress.set(true);
|
||||
|
|
|
|||
|
|
@ -65,7 +65,14 @@
|
|||
--space-5: 20px;
|
||||
--z-modal-backdrop: 10020;
|
||||
--z-modal: 10021;
|
||||
--z-confirm: 10031;
|
||||
/*
|
||||
* Dialogs that are opened from inside another dialog: a confirmation, and the provider/model
|
||||
* picker. At --z-modal they tied with the modal that opened them and lost on DOM order, so the
|
||||
* picker for "Evaluate impact with LLM" opened behind the report it was asked for and read as a
|
||||
* button that did nothing.
|
||||
*/
|
||||
--z-dialog-over-modal: 10031;
|
||||
--z-confirm: var(--z-dialog-over-modal);
|
||||
}
|
||||
|
||||
/* ── Semantic surface colours (hard-coded) ──────────────────────
|
||||
|
|
|
|||
Loading…
Reference in New Issue