Skip to content

[Website] Add error notices and delete confirmation modal to site management#3454

Open
bgrgicak wants to merge 10 commits intotrunkfrom
update/sites-api-feedback
Open

[Website] Add error notices and delete confirmation modal to site management#3454
bgrgicak wants to merge 10 commits intotrunkfrom
update/sites-api-feedback

Conversation

@bgrgicak
Copy link
Copy Markdown
Collaborator

@bgrgicak bgrgicak commented Apr 1, 2026

Motivation for the change, related issues

Follow-up to PR feedback on #3401. The PlaygroundSitesAPI methods throw descriptive errors for MCP consumers, but UI components were silently swallowing them. Additionally, site deletion used window.confirm, which is inconsistent with the rest of the modal-based UI and blocks the main thread.

Implementation details

Error notices in modals

  • Rename modal: catches errors from sitesAPI.rename() and displays them in a <Notice> component.
  • Save modal: surfaces the actual error message (instead of a generic string) and moves the error notice above the action buttons for visibility.
  • Delete modal (new): catches errors from sitesAPI.delete() and displays them inline.

Delete confirmation modal

  • Replaces window.confirm with a new DeleteSiteModal component, consistent with the existing rename/save modal pattern.
  • Adds DELETE_SITE modal slug and siteSlugToDelete UI state to the Redux store.
  • Both the site info panel and saved playgrounds overlay now dispatch the modal instead of calling window.confirm.

API documentation

  • Adds JSDoc comments with @param, @returns, and @throws tags to every method on the PlaygroundSitesAPI interface.

Minor cleanups

  • Inlines getActiveSiteOrThrow — each call site now checks and throws with a context-specific message ("No active site selected").
  • persistTemporarySite now lets showDirectoryPicker errors propagate instead of silently returning, so callers can handle cancellation.
  • Removes post-persist storage === 'none' safety-net checks that are no longer needed.

Testing Instructions

Testing Instructions

  1. In save-site-modal/index.tsx, add throw new Error('test') before the persistTemporarySite call in handleSubmit
  2. Open the Playground website and try saving a temporary site — confirm the error appears as a notice inside the modal
  3. Repeat for rename-site-modal/index.tsx (before sitesAPI.rename()) and delete-site-modal/index.tsx (before sitesAPI.delete())
  4. Remove the temporary throw statements
  5. Save, rename, and delete a site — confirm each operation succeeds and the modal closes
Screenshot 2026-04-01 at 16 39 38 Screenshot 2026-04-01 at 16 39 50 Screenshot 2026-04-01 at 16 40 44

@bgrgicak bgrgicak marked this pull request as ready for review April 1, 2026 20:58
@bgrgicak bgrgicak requested review from a team, adamziel, ashfame and Copilot April 1, 2026 20:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 improves site-management UX by surfacing API errors inside modals and replacing blocking window.confirm deletion with a dedicated modal flow.

Changes:

  • Add inline error notices to rename/save/delete site modals so API errors aren’t silently swallowed.
  • Replace window.confirm with a new DeleteSiteModal and Redux UI state to track which site is being deleted.
  • Add JSDoc (@param, @returns, @throws) documentation across the PlaygroundSitesAPI interface.

Reviewed changes

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

Show a summary per file
File Description
packages/playground/website/src/lib/state/redux/slice-ui.ts Adds DELETE_SITE modal slug and siteSlugToDelete UI state/action to support delete modal flow.
packages/playground/website/src/lib/state/redux/site-management-api-middleware.ts Adds API JSDoc and updates error/guard behavior for active-site operations and deletion messaging.
packages/playground/website/src/lib/state/redux/persist-temporary-site.ts Lets directory picker errors propagate so callers can show cancellation/errors in UI.
packages/playground/website/src/components/site-manager/site-info-panel/index.tsx Switches delete action from window.confirm + API call to opening the delete modal with selected slug.
packages/playground/website/src/components/saved-playgrounds-overlay/index.tsx Switches delete action to open delete modal (instead of confirming + calling API directly).
packages/playground/website/src/components/save-site-modal/index.tsx Displays real error messages via <Notice> and moves error notice above action buttons.
packages/playground/website/src/components/rename-site-modal/index.tsx Shows rename errors inline via <Notice> and unifies form layout via shared modal form CSS.
packages/playground/website/src/components/modal/style.module.css Adds shared .modal-form styling for consistent modal form layout.
packages/playground/website/src/components/layout/index.tsx Wires DELETE_SITE slug to render the new DeleteSiteModal.
packages/playground/website/src/components/delete-site-modal/index.tsx Introduces a delete confirmation modal with inline error handling.

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

bgrgicak and others added 2 commits April 7, 2026 11:49
When the DELETE_SITE modal is active but the target site no longer
exists (e.g. stale URL), dispatch closeModal actions so the URL param
is cleaned up. Also remove the storage === 'none' guard since both
callers already prevent temporary sites from reaching this modal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These modals require Redux state (siteSlugToDelete / siteSlugToRename)
that is not persisted in the URL, so restoring them from a stale URL
param would silently show nothing. Exclude them like save-site and
remove the now-unnecessary cleanup effect from DeleteSiteModal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants