Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix link modal not shown after access upgrade #12411

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Fix link modal not shown after access upgrade #12411

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7baa85a
Fix link modal not shown after access upgrade
toger5 Apr 11, 2024
2d2be47
dont shadow var
toger5 Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/components/views/rooms/RoomHeader/CallGuestLinkButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ export const CallGuestLinkButton: React.FC<{ room: Room }> = ({ room }) => {
// If the user cannot invite the Knocking is not given as an option.
canInvite,
}).finished.then(() => {
// we need to use the function here because the callback got called before the state was updated.
if (isRoomJoinable()) showLinkModal();
});
}
Expand Down
7 changes: 5 additions & 2 deletions src/hooks/room/useGuestAccessInformation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ export const useGuestAccessInformation = (room: Room): GuestAccessInformation =>
[canChangeJoinRule, isRoomJoinable, guestSpaUrl],
);

const isRoomJoinableFunction = (): boolean =>
room.getJoinRule() === JoinRule.Public || (joinRule === JoinRule.Knock && room.canInvite(room.myUserId));
const isRoomJoinableFunction = (): boolean => {
const joinRule = room.getJoinRule();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't you now shadowing joinRule? If it's the same thing, shouldn't this be a callback that depends on the value of joinRule?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AIUI not shadowing the joinRule is what caused the bug this PR aims to fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we need this to be computed on the fly.
This method is called from a callback that gets created when we show the first modal (asking to change permission rules)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, but if it's a different variable, it should generally have a different name rather than shadowing a variable in a higher scope. Wouldn't having it as a dependency make react do the right thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, but if it's a different variable, it should generally have a different name rather than shadowing a variable in a higher scope.

That is 100% correct. I see your point now.

I think having the stateVar here does not work because of this

    const shareClick = useCallback(() => {
        if (isRoomJoinable()) {
            showLinkModal();
        } else {
            // the room needs to be set to public or knock to generate a link
            Modal.createDialog(JoinRuleDialog, {
                room,
                // If the user cannot invite the Knocking is not given as an option.
                canInvite,
            }).finished.then(() => {
                if (isRoomJoinable()) showLinkModal();
            });
        }
    }, [isRoomJoinable, showLinkModal, room, canInvite]);

this line if (isRoomJoinable()) showLinkModal(); would not update the state variable even if it is a dependency since it would capture the variable while it is awaiting.

return joinRule === JoinRule.Public || (joinRule === JoinRule.Knock && room.canInvite(room.myUserId));
};

return { canInviteGuests, guestSpaUrl, isRoomJoinable: isRoomJoinableFunction, canInvite };
};
Loading