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

feat: Added Avatars and profile pictures in userAutoComplete #8892

Conversation

noufalrahim
Copy link
Contributor

Proposed Changes

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • [
    Screenshot from 2024-10-23 11-39-54
    Screenshot from 2024-10-23 11-40-58
    ] Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

@noufalrahim noufalrahim requested a review from a team as a code owner October 23, 2024 06:11
Copy link

netlify bot commented Oct 23, 2024

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 0b44cfa
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/6718fc7c256c85000839909e
😎 Deploy Preview https://deploy-preview-8892--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bodhish
Copy link
Member

bodhish commented Oct 23, 2024

image This looks broken

@noufalrahim
Copy link
Contributor Author

Sorry, I didn't understand..how is it broken..

@rithviknishad
Copy link
Member

image

enlarging the image such that the optionDescription (i.e. 'Doctor - @rithviknishad-doctor') comes to the right of the image would be a better UI

@noufalrahim
Copy link
Contributor Author

Screenshot from 2024-10-23 15-15-02
is this ok?

@rithviknishad
Copy link
Member

rithviknishad commented Oct 23, 2024

Better. The online dot could come to the bottom right of the avatar. Also prefix @ to the username

@noufalrahim
Copy link
Contributor Author

Screenshot from 2024-10-23 15-25-36
How is this?

@rithviknishad
Copy link
Member

Yup!

@noufalrahim
Copy link
Contributor Author

Updated!

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Oct 23, 2024
Copy link

👋 Hi, @noufalrahim,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

Comment on lines 83 to 85
optionImage={(option: UserBareMinimum | UserModel) =>
(option as UserModel).read_profile_picture_url || ""
}
Copy link
Member

Choose a reason for hiding this comment

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

if read_profile_picture_url is not present in UserBareMinimum but backend provides it, then add then move the read_profile_picture_url to UserBareMinimum instead.

Suggested change
optionImage={(option: UserBareMinimum | UserModel) =>
(option as UserModel).read_profile_picture_url || ""
}
optionImage={(option: UserBareMinimum) => option.read_profile_picture_url}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done..

<div className="w-full">
<div className="flex w-full justify-between">
<span>
{"@"}
Copy link
Member

Choose a reason for hiding this comment

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

@noufalrahim like said previously, this is a reusable component. Modifications made here will affect all other places. @ prefix is only supposed to be present for UserAutocompleteFormField not any other usages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So where am I supposed to add "@"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can I add it in utils formatName?
Screenshot from 2024-10-23 16-57-05

Copy link
Member

Choose a reason for hiding this comment

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

The username should have the prefix @, not the name

Copy link
Member

@rithviknishad rithviknishad Oct 23, 2024

Choose a reason for hiding this comment

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

You can add it in optionDescription of the UserAutocompleteFormField right?

image

@noufalrahim
Copy link
Contributor Author

I have a doubt regarding conflict.. How to resolve it..

@noufalrahim noufalrahim reopened this Oct 23, 2024
@rithviknishad
Copy link
Member

merge latest develop into this branch and clear conflicts from the editor and push the merge commit.

@noufalrahim
Copy link
Contributor Author

Opened another PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes required merge conflict pull requests with merge conflict
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show user avatar in User Autocomplete and User list page
3 participants