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

[$250] Chat - Sending emoji message with markdown, top half is cut off #53546

Open
2 of 8 tasks
izarutskaya opened this issue Dec 4, 2024 · 14 comments
Open
2 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Dec 4, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 53504
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
If this was caught during regression testing, add the test name, ID and link from TestRail: N
Email or phone of affected tester (no customers): N
Issue reported by: Applause Internal Team

Action Performed:

  1. Launch app
  2. Open a chat
  3. Send a emoji with markdown 😁😁

Expected Result:

Sending emoji message with markdown, top half must not be cut off.

Actual Result:

Sending emoji message with markdown, top half is cut off.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6683911_1733292611678.Screenrecorder-2024-12-04-11-36-07-266.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021864806387165915640
  • Upwork Job ID: 1864806387165915640
  • Last Price Increase: 2024-12-05
  • Automatic offers:
    • ZhenjaHorbach | Reviewer | 105276786
Issue OwnerCurrent Issue Owner: @bernhardoj
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 4, 2024
Copy link

melvin-bot bot commented Dec 4, 2024

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@Christinadobrzyn
Copy link
Contributor

Asking if this can be fixed as part of - #53324

@bernhardoj
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Emoji is cut off when it contains space and wrapped in a markdown.

What is the root cause of that problem?

This issue is similar to #43394 and #47899. The issue here is that, when the text only contains emoji, we apply a bigger font size and line height. However, in this specific case, there is a whitespace that has the default font size and line height which causes this issue. (this root cause is the same as the other 2 issues above)

What changes do you think we should make in order to solve the problem?

We need to apply a bigger line height to the whole message when the message contains only emojis. Then, we can remove the other solutions from #43394 and #47899. To do this:

  1. Pass containsOnlyEmojis to RenderCommentHTML component.
    <RenderCommentHTML
    source={source}
<RenderCommentHTML
    containsOnlyEmojis={containsOnlyEmojis}
  1. In RenderCommentHTML, if containsOnlyEmojis, then we add the islarge attribute.
    function RenderCommentHTML({html, source}: RenderCommentHTMLProps) {
    const commentHtml = source === 'email' ? `<email-comment>${html}</email-comment>` : `<comment>${html}</comment>`;
    return <RenderHTML html={commentHtml} />;
const commentHtml = source === 'email' ? `<email-comment ${containsOnlyEmojis ? 'islarge' : ''}>${html}</email-comment>` : `<comment ${containsOnlyEmojis ? 'islarge' : ''}>${html}</comment>`;
  1. In BaseHTMLEngineProvider, add the emoji-only line-height style if islarge is defined.
    comment: HTMLElementModel.fromCustomModel({
    tagName: 'comment',
    mixedUAStyles: {whiteSpace: 'pre'},
    contentModel: HTMLContentModel.block,
    }),
    'email-comment': HTMLElementModel.fromCustomModel({
    tagName: 'email-comment',
    mixedUAStyles: {whiteSpace: 'normal'},
    contentModel: HTMLContentModel.block,
    }),
getMixedUAStyles: (tnode) => {
    if (tnode.attributes.islarge === undefined) {
        return {whiteSpace: 'pre'}; // normal for email-comment
    }
    return {whiteSpace: 'pre', ...styles.onlyEmojisTextLineHeight};
},
  1. Then, remove the islarge from edited and blockquote tag solution

    const editedTag = fragment?.isEdited ? `<edited ${styleAsDeleted ? 'deleted' : ''} ${containsOnlyEmojis ? 'islarge' : ''}></edited>` : '';
    const htmlWithDeletedTag = styleAsDeleted ? `<del>${html}</del>` : html;
    let htmlContent = htmlWithDeletedTag;
    if (containsOnlyEmojis) {
    htmlContent = Str.replaceAll(htmlContent, '<emoji>', '<emoji islarge>');
    htmlContent = Str.replaceAll(htmlContent, '<blockquote>', '<blockquote isemojisonly>');

  2. Since edited has a custom renderer, we should remove the islarge logic from the component too

    const isLarge = !!(tnode.attributes.islarge !== undefined);
    return (
    <Text style={isLarge && styles.onlyEmojisTextLineHeight}>

and add fontSize={variables.fontSizeSmall} so our Text component won't apply a default line-height to it.

if (!componentStyle.lineHeight && componentStyle.fontSize === variables.fontSizeNormal) {
componentStyle.lineHeight = variables.fontSizeNormalHeight;
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A, it's a UI visual bug.

@Christinadobrzyn Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label Dec 5, 2024
@melvin-bot melvin-bot bot changed the title Chat - Sending emoji message with markdown, top half is cut off [$250] Chat - Sending emoji message with markdown, top half is cut off Dec 5, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021864806387165915640

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 5, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ZhenjaHorbach (External)

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Dec 6, 2024

@bernhardoj
Thanks for your proposal !
Could you create a draft PR please so that I don't miss anything ?

@bernhardoj
Copy link
Contributor

Here is the draft PR: #53707

@ZhenjaHorbach
Copy link
Contributor

Here is the draft PR: #53707

Thanks !
I'll check on Monday

@ZhenjaHorbach
Copy link
Contributor

@bernhardoj
Thanks for your proposal !
Your proposal makes sense and works well !

🎀👀🎀 C reviewed

Copy link

melvin-bot bot commented Dec 9, 2024

Triggered auto assignment to @youssef-lr, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 10, 2024
Copy link

melvin-bot bot commented Dec 10, 2024

📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Dec 11, 2024
@bernhardoj
Copy link
Contributor

PR is ready. Left some questions on the PR

cc: @ZhenjaHorbach

@Christinadobrzyn
Copy link
Contributor

monitoring PR #53707

1 similar comment
@Christinadobrzyn
Copy link
Contributor

monitoring PR #53707

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants