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] Added Bio Structure for UserCard, rendering Skeleton View on loading Instead of [Object][Object] #20305

Merged
merged 5 commits into from
Feb 2, 2021

Conversation

yash-rajpal
Copy link
Member

@yash-rajpal yash-rajpal commented Jan 21, 2021

  • I have read the Contributing Guide
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Proposed changes (including videos or screenshots)

Added Bio Structure for rendering Skeleton View on loading UserCard.

Issue(s)

fix #20283

Steps to test or reproduce

Check the Issue

Further comments

Until UserCard received bio, it was passing Skeleton as content to Markdown, so it was returning [Object][Object], which was shown to user. So Made one more entity called as bioStructure, now if bio is present, bio will be passed to markdown and everything would work as it is supposed to, but if bio is not present, bioStructure would be rendered as skeletonView.

@yash-rajpal
Copy link
Member Author

I hope the change is visible in GIF.
bio-structure-after

@pratyaksh123
Copy link

You could also do this way 👍
{ bio && ( <Info withTruncatedText={false} style={clampStyle} height='x60'> {typeof bio === 'string' ? <MarkdownText content={bio} /> : bio} </Info> )}

@ggazzo ggazzo requested a review from MartinSchoeler January 22, 2021 12:28
@ggazzo ggazzo added this to the 3.11.0 milestone Jan 22, 2021
Copy link
Contributor

@MartinSchoeler MartinSchoeler left a comment

Choose a reason for hiding this comment

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

Requested some changes, pratyaksh123 have some neat ideas to handling this. Thanks!

@@ -81,7 +82,7 @@ const UserCard = forwardRef(({
{ customStatus && <Info>{customStatus}</Info> }
<Roles>{roles}</Roles>
<Info>{localTime}</Info>
{ bio && <Info withTruncatedText={false} style={clampStyle} height='x60'><MarkdownText content={bio}/></Info> }
{bio !== undefined ? bio && <Info withTruncatedText={false} style={clampStyle} height='x60'><MarkdownText content={bio}/></Info> : <Info withTruncatedText={false} style={clampStyle} height='x60'>{bioStructure}</Info>}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe instead of doing this ternary, we can check the type of the bio variable inside of the info element?

@yash-rajpal
Copy link
Member Author

@MartinSchoeler Thanks, will do changes asap.

Copy link
Contributor

@MartinSchoeler MartinSchoeler left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@yash-rajpal
Copy link
Member Author

@ggazzo Please review. :)

@MartinSchoeler
Copy link
Contributor

@yash-rajpal please be patient, we are currently on a feature-freeze so pull requests that are not directly related to the ones done in the current release, can only be merged on the next release (don't worry we are almost at the end of the featrue-freeze).

Mentioning other members of the team will not make it go faster 🙂

Thanks!

@ggazzo ggazzo merged commit 63a8e14 into RocketChat:develop Feb 2, 2021
@sampaiodiego sampaiodiego mentioned this pull request Feb 28, 2021
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.

[BUG] Improper rendering of bio in UserCard
4 participants