Skip to content
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

fix: Made UserClickable opening userInfo #700

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

thepiyush-303
Copy link

@thepiyush-303 thepiyush-303 commented Dec 19, 2024

Brief Title

Acceptance Criteria fulfillment

  • Task 1
  • Task 2
  • Task 3

Fixes # (issue)
#699

Screencast.from.2024-12-19.14-04-17.webm

Video/Screenshots

PR Test Details

Note: The PR will be ready for live testing at https://rocketchat.github.io/EmbeddedChat/pulls/pr-700 after approval. Contributors are requested to replace <pr_number> with the actual PR number.

@CLAassistant
Copy link

CLAassistant commented Dec 19, 2024

CLA assistant check
All committers have signed the CLA.

@thepiyush-303
Copy link
Author

Done.

Copy link
Collaborator

@Spiral-Memory Spiral-Memory left a comment

Choose a reason for hiding this comment

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

Hi @thepiyush-303
It is taking time to open sidebar, instead, open the user panel and inside that, show a throbber and load member details along with appropriate error if it fails

Take inspiration from how on userclick or avatar click, it shows userinfo

@thepiyush-303
Copy link
Author

Hi @thepiyush-303 It is taking time to open sidebar, instead, open the user panel and inside that, show a throbber and load member details along with appropriate error if it fails

Take inspiration from how on userclick or avatar click, it shows userinfo

Done.
Please check if anything I am missing.

@Spiral-Memory
Copy link
Collaborator

Hey @thepiyush-303
I don't think you've implemented the feedback.

@Spiral-Memory
Copy link
Collaborator

Let me repeat my feedback..

On userclick (except here and all), open the sidebar first, show throbber, go ahead and make a request.. if success display user detail, otherwise display failed to fetch user info with the same design we have implemented in other sidepanels

@thepiyush-303
Copy link
Author

Are you talking about @mentions click then open sidebar?

@Spiral-Memory
Copy link
Collaborator

I think you're making user detail request, before opening the sidepanel.. first open it.. then make a request..

It is taking time to open sidepanel after click.. n if request is failing, the sidepanel isn't opening

@thepiyush-303
Copy link
Author

I got you I will fix it.

@thepiyush-303
Copy link
Author

@Spiral-Memory
I tried finding the issue about userInfo is fetched before component renders but I could not find it happening I think it is working fine as your feedback.
If I am missing something then please tell me.
Screencast from 2024-12-26 18-49-10.webm

@Spiral-Memory
Copy link
Collaborator

Hi @thepiyush-303

I am facing two issues with this implementation. Could you please help address them?

  1. Once a user is opened, clicking on another user does not open their details.
  2. When an error needs to be displayed, please ensure the UI remains consistent. For example, display the error in the center with an icon or proper formatting, instead of showing a raw error message to the user.

I am attaching a video and a photo for your reference.

2025-01-01.18-27-14.mp4

image

@thepiyush-303
Copy link
Author

Screenshot from 2025-01-02 01-25-02
I fix the error page now it looks like this.

@thepiyush-303
Copy link
Author

I am facing two issues with this implementation. Could you please help address them?

Once a user is opened, clicking on another user does not open their details.
When an error needs to be displayed, please ensure the UI remains consistent. For example, display the error in the center with an icon or proper formatting, instead of showing a raw error message to the user.

@Spiral-Memory Both issues are fixed.
Thanks for you response.

Screencast.from.2025-01-02.02-38-18.webm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants