Skip to content

refactor(frontend): share one dialog for confirm-then-act flows - #939

Merged
abdulrafey1 merged 10 commits into
mainfrom
rafe/confirm-dialog
Oct 26, 2026
Merged

refactor(frontend): share one dialog for confirm-then-act flows#939
abdulrafey1 merged 10 commits into
mainfrom
rafe/confirm-dialog

Conversation

@abdulrafey1

@abdulrafey1 abdulrafey1 commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Stacked on the PXC delete frontend PR (rafe/pxc-delete-frontend), which adds the dialog this extracts from.

What

Three screens each built their own confirm-then-act dialog: deleting a PXC activity, removing a whitelist entry, and disconnecting Slack. They now share one ConfirmDialog, so the pending state, dismiss rules and failure display behave the same everywhere.

Changes

  • feat(frontend): add a shared dialog that confirms before an action. components/ui/ConfirmDialog.tsx:
    • Runs an async onConfirm with a spinner on the confirm button.
    • Both buttons are disabled and Escape or an overlay click is ignored while it runs.
    • A failure is logged and keeps the dialog open with errorMessage.
  • refactor(frontend): confirm pxc activity deletes with the shared dialog. A 404 still counts as deleted.
  • refactor(frontend): confirm whitelist removal with the shared dialog. The deleting reducer state is gone.
  • refactor(frontend): confirm slack disconnect with the shared dialog. Adds the card's first tests.

How to Test

  1. make test.frontend.
  2. Activities: delete an activity. Cancel does nothing, confirm removes the row, and Escape is ignored while it runs.
  3. Admin > Email Whitelist: remove an entry. To see the failure path, stop the backend before confirming. The dialog stays open with "Failed to remove the entry."
  4. Slack: disconnect a connected workspace. The dialog shows "Disconnecting" while it runs and closes once it is done. With the backend stopped it stays open with "Failed to disconnect Slack."

Notes

  • Behaviour change (whitelist and Slack): a failure now keeps the dialog open with a fixed message.
    • Before, the dialog closed and the server's message appeared above the list or card.
    • The Slack dialog also stays open while the disconnect runs. Before, it closed first and the card's button showed the spinner.
  • The whitelist dialog now uses the default dialog width instead of its own narrower one.
  • No migrations, env vars or dependency changes.

This description was written with the assistance of an LLM (Claude).

🤖 Generated with Claude Code

@abdulrafey1
abdulrafey1 added this pull request to stack #907 October 21, 2026 11:56
@abdulrafey1 abdulrafey1 mentioned this pull request Oct 21, 2026
Merged
Base automatically changed from rafe/pxc-delete-frontend to main October 21, 2026 11:58
abdulrafey1 and others added 4 commits October 21, 2026 16:58
@abdulrafey1 @claude
feat(frontend): add a shared dialog that confirms before an action
f77ec5f 
ConfirmDialog runs an async action with a pending state, cannot be
dismissed while it runs, and stays open with an error when it fails.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1 @claude
refactor(frontend): confirm pxc activity deletes with the shared dialog
8a299ff 
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1 @claude
refactor(frontend): confirm whitelist removal with the shared dialog
0524919 
A failed removal now keeps the dialog open with an error, instead of
closing it and reporting the failure above the list.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1 @claude
refactor(frontend): confirm slack disconnect with the shared dialog
00735b0 
The dialog now stays open while the disconnect runs, and a failure keeps
it open with an error instead of reporting it above the card.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1 abdulrafey1 self-assigned this Oct 21, 2026
zaira-bibi
zaira-bibi requested changes Oct 22, 2026
View reviewed changes
Comment thread frontend/components/ui/ConfirmDialog.tsx Outdated
Comment thread frontend/plugins/slack/components/SlackConnectionCard.tsx
abdulrafey1 and others added 3 commits October 22, 2026 14:40
@abdulrafey1 @claude
fix(frontend): log confirm dialog failures under a fixed message
541b926 
The title is translated and can hold user content, such as a pxc
activity's name, so the same failure logged differently per locale.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1 @claude
fix(frontend): show an error when a confirmed action has no token
481a823 
Returning early counted as success to the confirm dialog, so a missing
token showed a spinner, did nothing and reported no error.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1
Merge remote-tracking branch 'origin/main' into rafe/confirm-dialog
f7153a8 
# Conflicts:
#	frontend/tests/app/dashboard/admin/whitelist/WhitelistPage.test.tsx
hamza-56
hamza-56 reviewed Oct 22, 2026
View reviewed changes

@hamza-56 hamza-56 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.

1 point:

Comment thread frontend/app/dashboard/admin/whitelist/WhitelistPage.tsx
abdulrafey1 and others added 2 commits October 23, 2026 12:41
@abdulrafey1 @claude
fix(frontend): show the server's reason when a confirmed action fails
a4075e5 
A failed whitelist removal, Slack disconnect or pxc delete showed only a
fixed string, so an admin hitting a permission error or a stale entry got no
hint why. The dialog now shows the message of an ApiRequestError, which the
backend sends in the UI's language, and keeps the fixed string for any other
error. disconnectSlack now rejects with the ApiRequestError itself so its
dialog can tell it apart.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1 @claude
fix(frontend): translate the network error message
f4a7cab 
A request that never reached the server showed "Unable to connect to server"
in English whatever the UI language, now that confirm dialogs show API error
messages. The message comes from the active catalog, which LocaleProvider
records next to the active locale, so code outside React can translate.
i18n-check cannot see keys used outside React's hooks, so the key is declared
with an i18n-check marker comment.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1
abdulrafey1 requested a review from hamza-56 October 23, 2026 07:57
@abdulrafey1 @claude
Merge remote-tracking branch 'origin/main' into rafe/confirm-dialog
5c94a33 
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
zaira-bibi
zaira-bibi approved these changes Oct 26, 2026
View reviewed changes

@zaira-bibi zaira-bibi 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.

good to go!

@abdulrafey1
abdulrafey1 merged commit 7c07a35 into main Oct 26, 2026
10 checks passed
@abdulrafey1
abdulrafey1 deleted the rafe/confirm-dialog branch October 26, 2026 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zaira-bibi zaira-bibi zaira-bibi approved these changes
@hamza-56 hamza-56 Awaiting requested review from hamza-56

Assignees

@abdulrafey1 abdulrafey1

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@abdulrafey1 @hamza-56 @zaira-bibi