appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
33.95k stars 3.66k forks source link

[Bug]: Share modal in view mode opens up multiple modals and appears glitchy #14601

Open ramsaptami opened 2 years ago

ramsaptami commented 2 years ago

Is there an existing issue for this?

Description

Share modal in view mode opens up multiple modals and appears glitchy. This does not happen all the time but this behaviour can be observed by repeating the action a couple of times

Steps To Reproduce

  1. Go to view mode of any app and click on the share button to see a glithcy behaviour as seen in the video

https://www.loom.com/share/c66bb2e72b1440a3b692867d3b6aebe5glitchy

Public Sample App

No response

Version

Cloud

nzidol commented 1 year ago

Happens both in View and Edit mode. When running in Cypress you can observe what is happening to the form.

  1. Click on "share" triggers get 200 /api/v1/workspaces image
  2. put 200 /api/v1/layouts/..../pages image
  3. get 200 /api/v1/workspaces/.../members image
  4. get 200 /api/v1/workspaces/.../permissionsgroup image

The relevant form is in app/client/src/ce/pages/workspace/WorkspaceInviteUserForm.tsx.

Solution could be to either:

nzidol commented 1 year ago

One aspect looks to be the use of React useEffect in WorkspaceInviteUserForm:

useEffect(() => {
   if (!isAclFlow) {
      fetchUser(props.workspaceId);
      fetchAllRoles(props.workspaceId);
      fetchCurrentWorkspace(props.workspaceId);
    }
  }, [props.workspaceId, fetchUser, fetchAllRoles, fetchCurrentWorkspace]);

Since the 3 api's will return at different times this will likely create the 3 redraws of the dialog that can be observed. Proposed solution is to use async/await the 3 api's:

useEffect(() => {
    const fetchAll3 = async () => {
        await fetchUser(props.workspaceId);
        await fetchAllRoles(props.workspaceId);
        await fetchCurrentWorkspace(props.workspaceId);
    };
    if (!isAclFlow) {
        fetchAll3();
    }
  }, [props.workspaceId, fetchUser, fetchAllRoles, fetchCurrentWorkspace]);

There is still an issue remaining where the WorkspaceInviteUserForm is added to the base dialog in client/src/pages/workspace/AppInviteUsersForm. This has it's own hook for an API request to /api/v1/workspaces before the WorkspaceInviteUserForm gets the members.

Tallon66 commented 1 year ago

@nzidol could I please work on this?

nzidol commented 1 year ago

Sure it's open source..... Personally I like to see multiple opinions and solutions, chance for everyone to learn....

Cheers

Ido

On Sat, 22 Oct 2022, 3:19 pm Tallon66, @.***> wrote:

@nzidol https://github.com/nzidol could I please work on this?

— Reply to this email directly, view it on GitHub https://github.com/appsmithorg/appsmith/issues/14601#issuecomment-1287586975, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH46HOKDCQONNDN4ZLFWM3WENFK5ANCNFSM5Y6AA7FQ . You are receiving this because you were mentioned.Message ID: @.***>

ramsaptami commented 1 year ago

@Tallon66 apologies for getting back so late. You can take this up if you're still up for it? I can assign it to you.

nzidol commented 1 year ago

Hi

Will be unavailable for any work on this for the next 3-4 months.

Sorry

Ido

On Tue, 31 Jan 2023, 5:42 pm ramsaptami, @.***> wrote:

@nzidol https://github.com/nzidol apologies for getting back so late. You can take this up if you're still up for it? I can assign it to you.

— Reply to this email directly, view it on GitHub https://github.com/appsmithorg/appsmith/issues/14601#issuecomment-1409746888, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH46HPU4WWVXMGLUBYGFNTWVCJ5DANCNFSM5Y6AA7FQ . You are receiving this because you were mentioned.Message ID: @.***>