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: Show placeholder message header for unavailable users #14928

Merged
merged 10 commits into from
Mar 30, 2023

Conversation

atomrc
Copy link
Contributor

@atomrc atomrc commented Mar 29, 2023

Will show a placeholder header when a user's metadata could not be fetched due to some federated backend being offline

image

Comment on lines +51 to +80
{sender.isExternal() && (
<span
className="message-header-icon-external with-tooltip with-tooltip--external"
data-tooltip={t('rolePartner')}
data-uie-name="sender-external"
>
<Icon.External />
</span>
)}

{sender.isFederated && (
<span
className="message-header-icon-guest with-tooltip with-tooltip--external"
data-tooltip={sender.handle}
data-uie-name="sender-federated"
>
<Icon.Federation />
</span>
)}

{sender.isDirectGuest() && (
<span
className="message-header-icon-guest with-tooltip with-tooltip--external"
data-tooltip={t('conversationGuestIndicator')}
data-uie-name="sender-guest"
>
<Icon.Guest />
</span>
)}
</>
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like these could all be combined into one function that returned the correct values from an object with a key that is equivalent to the sender?

Copy link
Contributor

Choose a reason for hiding this comment

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

something similar has been done here #14883 but not merged to dev yet. I think the idea is to come back and see if we can use the same reusable component later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, my plan is to use this one once it's merged. I'll have to identify if the contraints are really the same and go for it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

even #14883 seems a bit less DRY than id expect. the span component is duplicated several times.

Copy link
Contributor

Choose a reason for hiding this comment

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

anyway thats fine then.

@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Merging #14928 (d6884bf) into dev (54f33f2) will increase coverage by 0.04%.
The diff coverage is 65.51%.

@@            Coverage Diff             @@
##              dev   #14928      +/-   ##
==========================================
+ Coverage   42.94%   42.98%   +0.04%     
==========================================
  Files         626      627       +1     
  Lines       21320    21335      +15     
  Branches     4897     4908      +11     
==========================================
+ Hits         9155     9170      +15     
  Misses      10998    10998              
  Partials     1167     1167              

@atomrc atomrc merged commit aad7863 into dev Mar 30, 2023
@atomrc atomrc deleted the feat/placeholder-sender branch March 30, 2023 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants