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

test: Add tests to member message rendering #15969

Merged
merged 10 commits into from
Oct 11, 2023
Merged

Conversation

atomrc
Copy link
Contributor

@atomrc atomrc commented Oct 11, 2023

Description

This is a preparation for a refactoring of the MemberMessage ui component. The idea is to remove the dangerouslySetInnerHTML and have the rendering done in react directly.

This covers the feature by test so that I'm sure I'm not going to break things when refactoring

Checklist

  • PR has been self reviewed by the author;
  • Hard-to-understand areas of the code have been commented;
  • If it is a core feature, unit tests have been added;

@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #15969 (046b88a) into dev (e353f50) will increase coverage by 0.19%.
The diff coverage is 66.66%.

❗ Current head 046b88a differs from pull request most recent head 7b6706a. Consider uploading reports for the commit 7b6706a to get more accurate results

@@            Coverage Diff             @@
##              dev   #15969      +/-   ##
==========================================
+ Coverage   44.29%   44.48%   +0.19%     
==========================================
  Files         684      685       +1     
  Lines       22905    22899       -6     
  Branches     5204     5201       -3     
==========================================
+ Hits        10146    10187      +41     
+ Misses      11457    11398      -59     
- Partials     1302     1314      +12     

Comment on lines +20 to +26
import {MemberMessage as MemberMessageEntity} from 'src/script/entity/message/MemberMessage';
import {useKoSubscribableChildren} from 'Util/ComponentUtil';

export function MessageContent({message}: {message: MemberMessageEntity}) {
const {htmlCaption} = useKoSubscribableChildren(message, ['htmlCaption']);
return <p className="message-header-caption" dangerouslySetInnerHTML={{__html: htmlCaption}}></p>;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a follow up PR, this will be where the rendering will be done (no more dangerouslySetInnerHTML in there)

Comment on lines -137 to -151
case SystemMessageType.CONNECTION_ACCEPTED:
case SystemMessageType.CONNECTION_REQUEST: {
if (this.otherUser()) {
if (this.otherUser().isBlocked()) {
return t('conversationConnectionBlocked');
}

if (this.otherUser().isOutgoingRequest()) {
return '';
}
}

return t('conversationConnectionAccepted');
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was never reachable

Comment on lines -289 to -292
readonly showLargeAvatar = (): boolean => {
const largeAvatarTypes = [SystemMessageType.CONNECTION_ACCEPTED, SystemMessageType.CONNECTION_REQUEST];
return largeAvatarTypes.includes(this.memberMessageType);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Has been moved to the UI component (this doesn't belong here)

@atomrc atomrc changed the title refactor: Improve member system message rendering test: Add tests to member message rendering Oct 11, 2023
@atomrc atomrc marked this pull request as ready for review October 11, 2023 10:07
@atomrc atomrc requested review from otto-the-bot and a team as code owners October 11, 2023 10:07
@atomrc atomrc force-pushed the refactor/member-message-render branch from 176f78c to ebf7d79 Compare October 11, 2023 10:09
@atomrc atomrc merged commit b75e2ce into dev Oct 11, 2023
10 checks passed
@atomrc atomrc deleted the refactor/member-message-render branch October 11, 2023 11:47
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.

3 participants