Improve Invitation flow #9928

MrFlashAccount · 2024-05-13T08:16:05Z

Pull Request Description

Tl;dr

Closes: enso-org/cloud-v2#1186
This PR significantly improves the invitation UX and add ability to view/resend/copy/delete invitations

Demo Presentation

CleanShot.2024-05-14.at.16.28.40.mp4

Context:

This Change:

What the change does in the larger context? Specific details to highlight for review:

Redesign the Invitation dialog
Add the ability to edit emails after typing
Adds ability to use different separators: <space>, <semicolon>, <colon> or <newline>
Shows Invitation on the members page in settings.
Adds ability to remove, resend, copy or delete invitations
Improve the UI of dialogs, buttons

Test Plan:

Go over how you plan to test it. Your test plan should be more thorough the riskier the change is. For major changes, I like to describe how I E2E tested it and will monitor the rollout.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.

somebody1234

CR passed (other than minor issues raised in comments).
QA not yet done

app/ide-desktop/lib/dashboard/src/modals/InviteUsersModal/InviteUsersSuccess.tsx

app/ide-desktop/lib/dashboard/src/modals/InviteUsersModal/InviteUsersForm.tsx

app/ide-desktop/lib/dashboard/src/layouts/Settings.tsx

app/ide-desktop/lib/dashboard/src/tailwind.css

somebody1234 · 2024-05-15T10:28:16Z

app/ide-desktop/lib/dashboard/src/services/RemoteBackend.ts

@@ -254,15 +254,58 @@ export default class RemoteBackend extends Backend {

  /** Invite a new user to the organization by email. */
  override async inviteUser(body: backend.InviteUserRequestBody): Promise<void> {
-    const path = remoteBackendPaths.INVITE_USER_PATH


why remove path?

AFAIR I inlined the varible

yeah i mean that works, but separate path just makes it consistent with all the other endpoints in this file

app/ide-desktop/lib/dashboard/src/modals/InviteUsersModal/parseUserEmails.ts

somebody1234 · 2024-05-15T10:31:45Z

app/ide-desktop/lib/dashboard/src/modals/InviteUsersModal/InviteUsersSuccess.tsx

+    [locale]
+  )
+
+  const isUserOnMembersPage = decodeURIComponent(search) === `?${membersSearchParams}`


is there a reason this isn't passed as a boolean prop?

app/ide-desktop/lib/dashboard/src/components/Loader.tsx

somebody1234 · 2024-05-15T10:51:47Z

uhh

somebody1234 · 2024-05-15T10:52:21Z

QA ❌ ("CSS.highlights is undefined" crashing the entire app the moment i press any key in the invite modal)

somebody1234 · 2024-05-15T11:03:12Z

also remember to change Spaces to spaces

somebody1234 · 2024-05-15T11:25:00Z

QA ❌

Note: QA performed on Electron as the modal is currently still crashing on Firefox.
✔️ Emails in the "invite users" modal are now editable
✔️ Pending invites are visible in the "members" settings tab
⚠️ No warning when typing two of the same email in the input box. Maybe it shouldn't block sending invites, but IMO it should still be highlighted - e.g. in case the user copy-pasted an email because they plan to fill in the rest later (e.g. .group@example.com)
⚠️ "Spaces" -> "spaces, commas or semicolons" - or "spaces, , or ;"
❌ The font size of the success screen is way too big:
- ⚠️ The entire list is surrounded by quotes. Either each individual email should be surrounded by quotes, or there should be no quotes at all.
ℹ️ Cannot mass delete invites. Maybe we should have a button to clear all pending invites?
- Marked as ℹ️ because multi-select would require a major refactor and hence is probably out of scope.

❌ The modal cannot be closed by clicking on the dimmed background
❌ Username should be a separate column in the "members" table in the settings
❌ You should not be able to remove yourself from the organization
- ⚠️ Remove button should probably also always be red
❌ No dimmed background for "invite" modal in "members" settings tab
- ✔️ Clicking outside the modal hides the modal
⚠️ Ideally, deleted invitations would be removed from the list optimistically
- ⚠️ Deleting multiple invitations at once feels laggy

Minor UI nits:

⚠️ The font size for the title is tiny for some reason
⚠️ we should maybe make the close button red on hover? see the dashboard-122 branch (or related PR) for the specific color that is currently being used
The border radius of the buttons are a lot lower than those of the buttons everywhere else
- The height of the buttons is also inconsistent

MrFlashAccount · 2024-05-17T12:22:50Z

Addressing your QA remarks:

❌ Username should be a separate column in the "members" table in the settings

Why though?

❌ You should not be able to remove yourself from the organization

Currently, this button does nothing, I think we can tune up the behavior in this issue: https://github.com/enso-org/cloud-v2/issues/890

❌ No dimmed background for "invite" modal in "members" settings tab

Popovers do not dim background because basically, they are tooltips with rich content.

we should maybe make the close button red on hover

Not sure. I think generally we should use red color when we want to highlight irreversible actions like "Remove" or cancel. Closing the dialog isn't one of them.

somebody1234 · 2024-05-17T12:44:26Z

Username should be a separate column
why?

for consistency - none of our other tables have multiline cells

Currently, this button does nothing

we have a dedicated button elsewhere for deleting the current user, and it comes with plenty of warning

Popovers do not dim background

in that case we might want an actual modal instead - the "invite users" dialog is something that:

should take the user's focus, and
should either be addressed immediately (or closed immediately)

Closing the dialog isn't an irreversible action

oh, i meant the traffic light in the top left, so that it matches the macos traffic lights

MrFlashAccount · 2024-05-17T13:18:48Z

for consistency - none of our other tables have multiline cells

We'll have then an empty cell for invitations, which is strange.

in that case we might want an actual modal instead - the "invite users" dialog is something that:

Okay

oh, i meant the traffic light in the top left, so that it matches the macos traffic lights

Good idea 👍

MrFlashAccount · 2024-05-17T13:39:01Z

No warning when typing two of the same email in the input box. Maybe it shouldn't block sending invites, but IMO it should still be highlighted - e.g. in case the user copy-pasted an email because they plan to fill in the rest later (e.g. .group@example.com)

We just ignore duplicates, not sure we need to warn users...

MrFlashAccount · 2024-05-17T13:45:09Z

we have a dedicated button elsewhere for deleting the current user, and it comes with plenty of warning

Thanks for noticing, will take a look once when we'll work on this feature. Ideally, I think, we should merge the behavior.
But for invitations - I think it's safe if we delete without confirmation (We can create a new invitation anytime)

MrFlashAccount · 2024-05-17T13:45:34Z

I think I've addressed most of the issues and PR is ready for re-QA

PabloBuchu · 2024-05-22T12:13:56Z

Go to members page is not working (nothing happens)
Secondly if GET /invites is not ending with success it hangs Enso. I am on solo so backend responds with 403 but if I reload I dont see Enso only periodically re issued get invites request

Screen.Recording.2024-05-22.at.14.11.15.mov

PabloBuchu

see comment

somebody1234 · 2024-05-31T07:53:11Z

app/ide-desktop/lib/dashboard/src/tailwind.css

@@ -237,6 +237,8 @@

  --menu-entry-padding: 0.25rem;

+  --delete-color: rgba(243 24 10 / 87%);
+  --cta-color: rgb(250, 108, 8);


this causes issues with overriding opacity

MrFlashAccount self-assigned this May 13, 2024

MrFlashAccount added the CI: No changelog needed Do not require a changelog entry for this PR. label May 13, 2024

MrFlashAccount force-pushed the wip/sergeigarin/improve-invitation branch 2 times, most recently from d67d0af to 7b83a0b Compare May 14, 2024 12:58

MrFlashAccount marked this pull request as ready for review May 14, 2024 13:46

MrFlashAccount requested review from PabloBuchu, indiv0 and somebody1234 as code owners May 14, 2024 13:46

somebody1234 added x-new-feature Type: new feature request g-dashboard labels May 15, 2024

somebody1234 approved these changes May 15, 2024

View reviewed changes

MrFlashAccount force-pushed the wip/sergeigarin/improve-invitation branch 2 times, most recently from b5d518e to b8df31f Compare May 17, 2024 11:46

MrFlashAccount force-pushed the wip/sergeigarin/improve-invitation branch from aa03b2f to c0b9c43 Compare May 20, 2024 14:10

PabloBuchu approved these changes May 22, 2024

View reviewed changes

MrFlashAccount force-pushed the wip/sergeigarin/improve-invitation branch from c0b9c43 to 02c8640 Compare May 22, 2024 11:38

PabloBuchu requested changes May 22, 2024

View reviewed changes

PabloBuchu approved these changes May 22, 2024

View reviewed changes

PabloBuchu added the CI: Ready to merge This PR is eligible for automatic merge label May 22, 2024

MrFlashAccount added 13 commits May 22, 2024 19:38

Invite Dialog

9f8c061

close dialog on click outside

b462aab

åddress review issues

8b55998

address review questions

503d645

remove quotes

f821580

Improve success message

1a6a8d4

imrove ui/ux

53ed064

update lock file

384f0d1

fix lint

992ec7b

small fixes

4e16165

Pass status in error

993e976

fix lint

52d3745

fix btn alignment

e457804

MrFlashAccount force-pushed the wip/sergeigarin/improve-invitation branch from 50b3e3d to e457804 Compare May 22, 2024 16:40

MrFlashAccount removed the CI: Ready to merge This PR is eligible for automatic merge label May 22, 2024

fix navigation

2f82901

MrFlashAccount added the CI: Ready to merge This PR is eligible for automatic merge label May 22, 2024

mergify bot merged commit 58ae73d into develop May 22, 2024
36 checks passed

mergify bot deleted the wip/sergeigarin/improve-invitation branch May 22, 2024 18:26

somebody1234 reviewed May 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Invitation flow #9928

Improve Invitation flow #9928

MrFlashAccount commented May 13, 2024 •

edited

somebody1234 left a comment

somebody1234 May 15, 2024

MrFlashAccount May 15, 2024

somebody1234 May 15, 2024

somebody1234 May 15, 2024

somebody1234 commented May 15, 2024

somebody1234 commented May 15, 2024

somebody1234 commented May 15, 2024

somebody1234 commented May 15, 2024

MrFlashAccount commented May 17, 2024 •

edited

somebody1234 commented May 17, 2024

MrFlashAccount commented May 17, 2024

MrFlashAccount commented May 17, 2024

MrFlashAccount commented May 17, 2024

MrFlashAccount commented May 17, 2024

PabloBuchu commented May 22, 2024

PabloBuchu left a comment

somebody1234 May 31, 2024

Improve Invitation flow #9928

Improve Invitation flow #9928

Conversation

MrFlashAccount commented May 13, 2024 • edited

Pull Request Description

Tl;dr

Context:

This Change:

Test Plan:

Checklist

somebody1234 left a comment

Choose a reason for hiding this comment

somebody1234 May 15, 2024

Choose a reason for hiding this comment

MrFlashAccount May 15, 2024

Choose a reason for hiding this comment

somebody1234 May 15, 2024

Choose a reason for hiding this comment

somebody1234 May 15, 2024

Choose a reason for hiding this comment

somebody1234 commented May 15, 2024

somebody1234 commented May 15, 2024

somebody1234 commented May 15, 2024

somebody1234 commented May 15, 2024

MrFlashAccount commented May 17, 2024 • edited

somebody1234 commented May 17, 2024

MrFlashAccount commented May 17, 2024

MrFlashAccount commented May 17, 2024

MrFlashAccount commented May 17, 2024

MrFlashAccount commented May 17, 2024

PabloBuchu commented May 22, 2024

PabloBuchu left a comment

Choose a reason for hiding this comment

somebody1234 May 31, 2024

Choose a reason for hiding this comment

MrFlashAccount commented May 13, 2024 •

edited

MrFlashAccount commented May 17, 2024 •

edited