-
Notifications
You must be signed in to change notification settings - Fork 532
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
Added avatars to userlist and userAutoComplete #8903
Added avatars to userlist and userAutoComplete #8903
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
lgtm; but could you discard changes made to the lockfile?
… issues/ohcnetwork#8869/user-avatat-in-user-autocomplete-and-user-list
Done! |
Why is Cypress test failing? Is there anything that needs to be done from my end? |
@@ -21311,4 +21311,4 @@ | |||
} | |||
} | |||
} | |||
} | |||
} |
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.
Unrelated change, do we need this 🤔
@@ -80,8 +80,11 @@ export default function UserAutocomplete(props: UserSearchProps) { | |||
)} | |||
optionLabel={formatName} | |||
optionIcon={userOnlineDot} | |||
optionImage={(option: UserBareMinimum) => | |||
option.read_profile_picture_url || "" |
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.
why should we cast it to an empty string 🤔, we should be able to deal with an option right 🤔
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.
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.
you can make the optionImage
accept undefined right?
Opened another PR |
Why close and re-create PR? |
Merge conflict was there..so I undo everything, created a new branch and committed. |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist