Skip to content

fix: Add cancel button to invite user modal#2522

Open
finnar-bin wants to merge 3 commits into
stagefrom
fix/2521-add-invite-modal-cancel-button
Open

fix: Add cancel button to invite user modal#2522
finnar-bin wants to merge 3 commits into
stagefrom
fix/2521-add-invite-modal-cancel-button

Conversation

@finnar-bin

@finnar-bin finnar-bin commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Resolves #2521

Summary

  • Added a Cancel button to the CustomForm component inside the Invite User modal in the Accounts Users view
  • Wired the button to close the SweetAlert2 modal via MySwal.close(), giving users a way to dismiss the dialog without submitting

Test plan

  • Navigate to an instance's Users page in the Accounts UI
  • Click the invite user button to open the modal
  • Verify the Cancel button appears below the Submit button
  • Click Cancel and confirm the modal closes without sending an invite
  • Verify the Cancel button is disabled while the form is submitting
  • Check appearance in both light and dark mode

Preview

Screenshot_20260608_094625

@finnar-bin finnar-bin added the bug Something isn't working label Jun 8, 2026
@finnar-bin finnar-bin self-assigned this Jun 8, 2026
@finnar-bin finnar-bin changed the title Settings: Add cancel button to invite user modal fix: Add cancel button to invite user modal Jun 8, 2026
Comment thread src/views/accounts/instances/Users.js
Comment thread src/views/accounts/instances/Users.js Outdated
Comment thread src/views/accounts/instances/Users.js Outdated
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review summary

Small, focused change that does what it sets out to do — adds a Cancel button to the invite-user SweetAlert modal and wires it to MySwal.close(). Behavior looks correct: the button is disabled during submission, and closing the modal unmounts the form so transient state is discarded.

Findings

Code quality (non-blocking)

  • useTheme() is called inside CustomForm but sx already exposes theme via its callback — duplicating theme access in the same component reads as noise. Consolidate into the sx callback and drop the hook/import.
  • Conditional sx values use && short-circuit (e.g. theme.palette.mode === 'light' && 'white'), which evaluates to literal false in dark mode. MUI tolerates this, but a ternary or conditional spread makes intent explicit and avoids surprising CSS.
  • Hardcoded 'white' / 'black' bypass the palette. theme.palette.common.white / theme.palette.background.paper keep the button aligned with the theme.
  • The color prop flips between 'inherit' (light) and 'primary' (dark). In dark mode this gives the Cancel button a primary-blue tint that competes with the Submit button — consider a neutral color in both modes for a clearer secondary-action treatment.

Bugs / security / performance

  • No bugs, security concerns, or performance issues spotted. The change is purely additive UI.

Test plan

The manual test plan in the PR description covers the key paths. Worth also confirming:

  • Clicking Cancel while the form has partially filled fields discards them (i.e. reopening the modal shows a clean form).
  • Escape-to-dismiss / backdrop-click behavior of SweetAlert is unchanged.

@finnar-bin finnar-bin requested a review from agalin920 June 8, 2026 01:52
Comment thread src/views/accounts/instances/Users.js
Comment thread src/views/accounts/instances/Users.js
Comment thread src/views/accounts/instances/Users.js
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review summary

Small, well-scoped change that addresses the missing dismiss affordance. Two issues to address before merging, plus one minor nit — left inline:

  1. Cancel button is missing type="button" — without it, clicking Cancel can also submit the form (HTML default for buttons inside <form> is type="submit"). This can cause the invite to be sent even when the user is trying to dismiss the dialog. Other buttons in this codebase set this explicitly inside forms.
  2. Hardcoded white / grey[200] / black styles ignore the theme — the PR's own test plan asks to check dark mode, but these values force a light-mode appearance. Prefer MUI theme tokens (divider, action.hover) so the outlined variant works in both palettes.
  3. Nit: passing MySwal.close as a bare reference (line 323) is fine today but a thin wrapper (() => MySwal.close()) is safer if SweetAlert2 ever rebinds it.

No security or performance concerns. Functionality is straightforward once #1 is fixed.

Comment thread src/views/accounts/instances/Users.js
Comment thread src/views/accounts/instances/Users.js
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Summary

Scope is tight: 25 lines in one file, adding a Cancel button to the invite-user SweetAlert modal. The change is functional and the iteration history shows responsiveness to feedback (dropping the unused useTheme import, adding type="button" so the cancel doesn't get treated as a form submit).

Findings

  • Subtle: onCancel={MySwal.close} passes the React click event into Swal.close() as resolveValue. No caller awaits MySwal.fire(...) today so nothing breaks, but the commit-1 form () => MySwal.close() was safer. See inline comment.
  • Style consistency: The dark-mode conditionals were stripped in commit 2, leaving bgcolor: 'white' and color: 'black' hard-coded. The PR test plan still asks to verify dark mode, but the button now renders identically in both. Existing precedent in src/views/accounts/instances/Overview.js keeps the theme.palette.mode === 'light' conditionals. See inline comment.
  • Positive: Good use of disabled={formik.isSubmitting} so the cancel button can't race the submit. type="button" correctly prevents accidental form submission.
  • No security or performance concerns. The Cancel button is local to the modal and adds no new data flow.

Suggested test plan additions

  • Open the modal, type partial values into the form, click Cancel, reopen — confirm fields are blank (CustomForm remounts each time, so this should hold).
  • Verify the Cancel button is keyboard-reachable via Tab and triggers on Enter/Space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accounts bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accounts UI - Invite User modal does not have a cancel button

2 participants