-
Notifications
You must be signed in to change notification settings - Fork 11.7k
chore: apply useQuery
on InviteUsers
#35861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteUsersError.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteUsersLoading.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteUsersWithData.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteUsersWithData.tsx
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteUsersWithData.tsx
Outdated
Show resolved
Hide resolved
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #35861 +/- ##
===========================================
- Coverage 61.21% 61.21% -0.01%
===========================================
Files 3007 3011 +4
Lines 71472 71486 +14
Branches 16343 16345 +2
===========================================
+ Hits 43752 43758 +6
- Misses 24752 24760 +8
Partials 2968 2968
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
08c00c9
to
b025203
Compare
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteUsersWithData.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteUsersLoading.tsx
Outdated
Show resolved
Hide resolved
This comment was marked as duplicate.
This comment was marked as duplicate.
1 similar comment
This comment was marked as duplicate.
This comment was marked as duplicate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to have some snapshots and a11y tests in these screens
517b525
to
eacd3a2
Compare
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
This comment was marked as duplicate.
This comment was marked as duplicate.
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/EditInviteLink.tsx
Show resolved
Hide resolved
apps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/InviteLink.tsx
Show resolved
Hide resolved
|
||
const InviteUsersLoading = ({ onClose, onClickBack: onClickBackMembers }: InviteUsersProps): ReactElement => ( | ||
<InviteUsersWrapper onClose={onClose} onClickBack={onClickBackMembers}> | ||
<Skeleton w='full' /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if we try to render something similar to the ContextualbarSkeleton
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ContextualbarSkeleton
is loading the whole contextualbar - header title and content - and has no Back
or Close
buttons.
In this case we have all these info loaded already, so I decided to put a skeleton only on the content that is pending.
ARCH-1579
Proposed changes (including videos or screenshots)
In dev mode, invite links were being generated 2 times due to useEffect
useEffect
in favor ofuseQuery
Issue(s)
Steps to test or reproduce
Further comments
This pull request focuses on refactoring the
InviteUsers
functionality within the Rocket.Chat application. The main change involves applyinguseQuery
for data fetching in theInviteUsers
component. The update includes the removal of existing error handling and the Callout component, which may affect error visibility and user experience. To address error handling, a new component,InviteUsersError
, is introduced to manage and display errors during the user invitation process. Additionally, a new loading component,InviteUsersLoading
, is added to enhance the user interface during the invitation process. TheInviteUsersWithData
component is refactored to incorporate React Query for improved data management and state handling. While the code is generally well-structured, there are suggestions for further improvements in error handling and type safety across the new components.