Skip to content

Use toast notification for preview changes#415

Draft
RRosio wants to merge 3 commits into
mamba-org:mainfrom
RRosio:fix_pkg_preview
Draft

Use toast notification for preview changes#415
RRosio wants to merge 3 commits into
mamba-org:mainfrom
RRosio:fix_pkg_preview

Conversation

@RRosio

@RRosio RRosio commented Apr 25, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions

Copy link
Copy Markdown

Binder 👈 Launch a binder notebook on the branch RRosio/gator/fix_pkg_preview

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the package-change workflow to use toast notifications during the preview phase and to improve error reporting when applying changes.

Changes:

  • Add a “Previewing package changes” in-progress toast while computing dry-run previews.
  • Skip opening the preview dialog when the dry-run indicates no solver side effects, and proceed directly to apply.
  • Improve apply error toasts by showing a short first-line message with a “Show details” action.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.

File Description
yarn.lock Updates TypeScript patch resolution/checksum entries.
packages/common/src/packageActions.ts Adds preview toast lifecycle, reuses dry-run preview results, and enhances error toast details/actions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/common/src/packageActions.ts
Comment on lines 434 to +438
} else {
Notification.error((error as any).message);
Notification.emit(firstLine, 'error', {
autoClose: false,
actions: [
{

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the error path where toastId is still empty, the code emits an error toast but does not dismiss previewAllNotification, which can leave the in-progress preview toast lingering alongside the error. Dismiss previewAllNotification in this branch as well (ideally via a shared cleanup/finally).

Copilot uses AI. Check for mistakes.
Comment on lines +271 to +285
const removePreview = await pkgModel.dry_run_preview(
toRemove,
'remove',
theEnvironment
);
const updatePreview = await pkgModel.dry_run_preview(
toUpdate,
'update',
theEnvironment
);
const installPreview = await pkgModel.dry_run_preview(
toInstall,
'install',
theEnvironment
);

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three dry_run_preview calls are awaited sequentially, which increases latency compared to the previous behavior where the dialog loaded previews concurrently via Promise.all. Consider running these previews in parallel (e.g., await Promise.all([...])) and then using the results for has_side_effects checks / job data.

Suggested change
const removePreview = await pkgModel.dry_run_preview(
toRemove,
'remove',
theEnvironment
);
const updatePreview = await pkgModel.dry_run_preview(
toUpdate,
'update',
theEnvironment
);
const installPreview = await pkgModel.dry_run_preview(
toInstall,
'install',
theEnvironment
);
const [removePreview, updatePreview, installPreview] = await Promise.all([
pkgModel.dry_run_preview(toRemove, 'remove', theEnvironment),
pkgModel.dry_run_preview(toUpdate, 'update', theEnvironment),
pkgModel.dry_run_preview(toInstall, 'install', theEnvironment)
]);

Copilot uses AI. Check for mistakes.
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.

Use toast while resolving preview for direct package updates

2 participants