Skip to content

webapp: the settings dialogs stop holding on a spinner #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
pythonlearner1025 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

webapp: the settings dialogs stop holding on a spinner #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 51 additions & 33 deletions packages/webapp/src/AgentRulesPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,48 +76,66 @@ export function AgentRulesPicker({
});
};

const save = async () => {
const save = () => {
if (draft === null || busy) return;
const name = draft.name.trim();
if (name === '' || draft.content.trim() === '') return;
const precedingRules = rules;
const precedingValue = value;
const precedingDraft = draft;
const id = draft.id ?? crypto.randomUUID();
const optimistic: AgentRuleView = {
id,
name,
content: draft.content,
updatedAt: Date.now(),
builtIn: false,
};
setBusy(true);
setError(null);
try {
const id = draft.id ?? crypto.randomUUID();
// The PUT returns the canonical row, so the list is updated from it
// rather than re-fetched: one round trip, and no window where the save
// succeeded but the select cannot name what it just selected.
const { rule } = await client.putAgentRule(id, { name, content: draft.content });
setRules((current) => current.some((entry) => entry.id === rule.id)
? current.map((entry) => entry.id === rule.id ? rule : entry)
: [...current, rule]);
onChange(rule.id);
setDraft(null);
} catch (caught) {
setError(caught instanceof Error ? caught.message : 'The rule could not be saved.');
} finally {
setBusy(false);
}
setRules((current) => current.some((entry) => entry.id === id)
? current.map((entry) => entry.id === id ? optimistic : entry)
: [...current, optimistic]);
onChange(id);
setDraft(null);
void client.putAgentRule(id, { name, content: draft.content })
.then(({ rule }) => {
// The route answers with the normalized row, so the placeholder never
// becomes a second source of truth.
setRules((current) => current.map((entry) => entry.id === id ? rule : entry));
onChange(rule.id);
})
.catch((caught) => {
setRules(precedingRules);
onChange(precedingValue);
setDraft(precedingDraft);
setError(caught instanceof Error ? caught.message : 'The rule could not be saved.');
})
.finally(() => setBusy(false));
};

const remove = async () => {
const remove = () => {
if (draft === null || draft.id === null || busy) return;
const precedingRules = rules;
const precedingValue = value;
const precedingDraft = draft;
const removed = draft.id;
setBusy(true);
setError(null);
// The confirmation has done its job; a failure is reported in the editor
// behind it, not under a dialog still asking the same question.
// The confirmation has done its job; a rejection reopens the editor with
// its exact draft instead of leaving the destructive prompt on screen.
setConfirmingDelete(false);
try {
const removed = draft.id;
await client.deleteAgentRule(removed);
setRules((current) => current.filter((entry) => entry.id !== removed));
if (value === removed) onChange(null);
setDraft(null);
} catch (caught) {
setError(caught instanceof Error ? caught.message : 'The rule could not be deleted.');
} finally {
setBusy(false);
}
setRules((current) => current.filter((entry) => entry.id !== removed));
if (value === removed) onChange(null);
setDraft(null);
void client.deleteAgentRule(removed)
.catch((caught) => {
setRules(precedingRules);
onChange(precedingValue);
setDraft(precedingDraft);
setError(caught instanceof Error ? caught.message : 'The rule could not be deleted.');
})
.finally(() => setBusy(false));
};

return (
Expand All @@ -137,7 +155,7 @@ export function AgentRulesPicker({
id={selectId}
aria-label="Agent rules document"
value={value ?? ''}
disabled={disabled}
disabled={disabled || busy}
onChange={(event) => {
const next = event.currentTarget.value;
if (next === NEW_RULE_OPTION) {
Expand All @@ -156,7 +174,7 @@ export function AgentRulesPicker({
<button
className="webapp-action blueprint-agent-rules-edit"
type="button"
disabled={disabled}
disabled={disabled || busy}
onClick={() => openDraft(selected ?? builtIn)}
>
Edit
Expand Down
122 changes: 81 additions & 41 deletions packages/webapp/src/WorkspaceDetailsDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,59 +129,99 @@ export function WorkspaceDetailsDialog({
return () => { cancelled = true; };
}, [client, listMachineTypes, workspaceId]);

const saveSettings = useCallback(async (input: UpdateWorkspaceRequest) => {
try {
const { workspace: updated } = await client.updateWorkspace(workspaceId, input);
commitWorkspaceMutation({
type: 'workspace_settings_updated',
workspaceId,
settings: {
serverName: updated.name,
const saveSettings = useCallback((input: UpdateWorkspaceRequest) => {
const snapshot = {
serverName: workspace.serverName,
defaultMachineTypeId: workspace.defaultMachineTypeId,
autoProvision: workspace.autoProvision,
agentRuleId: workspace.agentRuleId,
updatedAt: workspace.updatedAt,
};
const optimistic = {
serverName: input.name ?? workspace.serverName,
defaultMachineTypeId: input.defaultMachineTypeId ?? workspace.defaultMachineTypeId,
autoProvision: input.autoProvision ?? workspace.autoProvision,
agentRuleId: input.agentRuleId === undefined ? workspace.agentRuleId : input.agentRuleId,
updatedAt: Date.now(),
};
commitWorkspaceMutation({
type: 'workspace_settings_updated',
workspaceId,
settings: optimistic,
});
return client.updateWorkspace(workspaceId, input)
.then(({ workspace: updated }) => {
commitWorkspaceMutation({
type: 'workspace_settings_updated',
workspaceId,
settings: {
serverName: updated.name,
defaultMachineTypeId: updated.defaultMachineTypeId,
autoProvision: updated.autoProvision,
agentRuleId: updated.agentRuleId,
updatedAt: updated.updatedAt,
},
});
return {
name: updated.name,
defaultMachineTypeId: updated.defaultMachineTypeId,
autoProvision: updated.autoProvision,
agentRuleId: updated.agentRuleId,
updatedAt: updated.updatedAt,
},
});
return {
name: updated.name,
defaultMachineTypeId: updated.defaultMachineTypeId,
autoProvision: updated.autoProvision,
agentRuleId: updated.agentRuleId,
};
} catch (caught) {
reportError(caught instanceof Error ? caught : new Error('The settings could not be saved.'), {
title: 'Couldn’t save workspace settings',
action: 'Saving settings for ' + workspace.title + '.',
workspaceId,
};
})
.catch((caught) => {
commitWorkspaceMutation({
type: 'workspace_settings_updated',
workspaceId,
settings: snapshot,
});
reportError(caught instanceof Error ? caught : new Error('The settings could not be saved.'), {
title: 'Couldn’t save workspace settings',
action: 'Saving settings for ' + workspace.title + '.',
workspaceId,
});
throw caught;
});
throw caught;
}
}, [client, commitWorkspaceMutation, reportError, workspace.title, workspaceId]);
}, [client, commitWorkspaceMutation, reportError, workspace, workspaceId]);

/** A repo write answers with the list it produced, so the panel shows what
* the server holds rather than what the browser hoped for. A remove answers
* 204, and the row the server agreed to delete is the one dropped here. */
/** An add stays visible while its write runs, then the response replaces the
* whole list. A remove remembers its index so a rejection restores order. */
const addRepo = (repo: string) => {
const pending: TemplateRepoView = { repo, private: false };
setRepos((current) => current.some((entry) => entry.repo === repo)
? current
: [...current, pending]);
void client.addWorkspaceRepo(workspaceId, { repo })
.then((response) => { setRepos(response.repos); })
.catch((caught) => reportError(caught, {
title: 'Couldn’t add repository',
action: `Adding ${repo} to ${workspace.title}.`,
workspaceId,
}));
.catch((caught) => {
setRepos((current) => current.filter((entry) => entry !== pending));
reportError(caught, {
title: 'Couldn’t add repository',
action: `Adding ${repo} to ${workspace.title}.`,
workspaceId,
});
});
};

const removeRepo = (repo: string) => {
const index = repos.findIndex((entry) => entry.repo === repo);
const removed = repos[index];
if (removed === undefined) return;
setRepos((current) => current.filter((entry) => entry.repo !== repo));
void client.removeWorkspaceRepo(workspaceId, repo)
.then(() => {
setRepos((current) => current.filter((entry) => entry.repo !== repo));
})
.catch((caught) => reportError(caught, {
title: 'Couldn’t remove repository',
action: `Removing ${repo} from ${workspace.title}.`,
workspaceId,
}));
.catch((caught) => {
setRepos((current) => {
if (current.some((entry) => entry.repo === repo)) return current;
const restored = [...current];
restored.splice(Math.min(Math.max(index, 0), restored.length), 0, removed);
return restored;
});
reportError(caught, {
title: 'Couldn’t remove repository',
action: `Removing ${repo} from ${workspace.title}.`,
workspaceId,
});
});
};

const changeMachineType = (member: WorkspaceMemberView, machineTypeId: string) => {
Expand Down
16 changes: 5 additions & 11 deletions packages/webapp/src/WorkspaceSettingsTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ export function WorkspaceSettingsTab({
onRemoveRepo: (repo: string) => void;
}) {
const [draft, setDraft] = useState<SettingsDraft>(() => draftFor(workspace));
const [saving, setSaving] = useState(false);
const changes = settingsChanges(workspace, draft);
const defaultMachine = machines.find(({ id }) => id === draft.defaultMachineTypeId);
return (
Expand All @@ -191,7 +190,6 @@ export function WorkspaceSettingsTab({
autoCapitalize="off"
autoCorrect="off"
spellCheck={false}
disabled={saving}
value={draft.name}
onChange={(event) => {
const name = event.currentTarget.value;
Expand All @@ -204,7 +202,6 @@ export function WorkspaceSettingsTab({
<WebAppSelectMenu
ariaLabel="Default machine type"
className="machine-type-select"
disabled={saving}
value={draft.defaultMachineTypeId}
options={machineTypeOptions(machines)}
onChange={(defaultMachineTypeId) =>
Expand All @@ -221,7 +218,6 @@ export function WorkspaceSettingsTab({
<input
type="checkbox"
aria-label="Provision a machine when a member is added"
disabled={saving}
checked={draft.autoProvision}
onChange={(event) => {
const autoProvision = event.currentTarget.checked;
Expand All @@ -234,7 +230,6 @@ export function WorkspaceSettingsTab({
<AgentRulesPicker
client={client}
value={draft.agentRuleId}
disabled={saving}
onChange={(agentRuleId) => setDraft((current) => ({ ...current, agentRuleId }))}
/>
{/* One Save for the whole form, so it belongs to neither section and
Expand All @@ -243,17 +238,16 @@ export function WorkspaceSettingsTab({
<button
className="webapp-action webapp-action--primary"
type="button"
disabled={changes === null || saving}
disabled={changes === null}
onClick={() => {
if (changes === null || saving) return;
setSaving(true);
if (changes === null) return;
const snapshot = draftFor(workspace);
void onSave(changes)
.then((canonical) => setDraft(canonical))
.catch(() => undefined)
.finally(() => setSaving(false));
.catch(() => setDraft(snapshot));
}}
>
{saving ? 'Saving…' : 'Save settings'}
Save settings
</button>
</div>
</>
Expand Down
Loading
Loading