Truncation fix for Lack of maximum height for the description field

[brandonhenry]

Details

Fixed Issues

$ https://github.com/Expensify/App/issues/37357 PROPOSAL: https://github.com/Expensify/App/issues/37357#issuecomment-1989729926

Tests

1. Open the chat room details page for a room with a long description that includes markdown formatting
2. Verify that the description is truncated to the specified max length (e.g. 100 characters)

• [x] Verify that the truncated description preserves valid markdown syntax
• [x] Verify that other key room details and options are visible on the screen and not pushed off by the description

Offline tests

1. Put the device in airplane mode to simulate being offline
2. Open the chat room details page for a room with a long description

• [x] Verify the truncated description is still displayed correctly while offline

QA Steps

1. In staging, create a new chat room and add a long description (over 100 characters) that includes bold, italic, links and other markdown syntax
2. Open the room details page

• [x] Verify the description is properly truncated to 100 characters
• [x] Verify the truncated text maintains valid markdown - no broken formatting or tags
• [x] Verify other key room details like Room name, participants, options menu etc are still visible

PR Author Checklist

• [x] I linked the correct issue in the ### Fixed Issues section above
• [x] I wrote clear testing steps that cover the changes made in this PR
  • [x] I added steps for local testing in the Tests section
  • [x] I added steps for the expected offline behavior in the Offline steps section
  • [x] I added steps for Staging and/or Production testing in the QA steps section
  • [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • [x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• [x] I included screenshots or videos for tests on all platforms
• [x] I ran the tests on all platforms & verified they passed on:
  • [x] Android: Native
  • [x] Android: mWeb Chrome
  • [x] iOS: Native
  • [x] iOS: mWeb Safari
  • [x] MacOS: Chrome / Safari
  • [x] MacOS: Desktop
• [x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• [x] I followed proper code patterns (see Reviewing the code)
  • [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • [x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • [x] I verified that comments were added to code that is not self explanatory
  • [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • [x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • [x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • [x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • [x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • [x] I verified the JSDocs style guidelines (in STYLE.md) were followed
• [x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• [x] I followed the guidelines as stated in the Review Guidelines
• [x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• [x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• [x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• [x] I verified that if a function's arguments changed that all usages have also been updated correctly
• [x] If any new file was added I verified that:
  • [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• [x] If a new CSS style is added I verified that:
  • [x] A similar style doesn't already exist
  • [x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• [x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• [x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• [x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• [x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • [x] I verified that all the inputs inside a form are aligned with each other.
  • [x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• [x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• [x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

[brandonhenry]

this is awaiting review over at expensify-common. PR

[melvin-bot[bot]]

@shubham1206agra Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[shubham1206agra]

@brandonhenry Can you create a temporary function here to test this flow?

[brandonhenry]

@shubham1206agra example added where the fix is meant to be applied. Used 300 as the limit

[brandonhenry]

@shubham1206agra any updates on this one 😄

[shubham1206agra]

@brandonhenry Can you merge main here?

[brandonhenry]

@shubham1206agra just merged it in

[brandonhenry]

@shubham1206agra let me know if you find anything on this, hopefully we can get this merged before Tuesday 🤞🏿

[shubham1206agra]

Not really. This approach has some weird bugs.

I am going through https://github.com/huang47/nodejs-html-truncate/blob/master/lib/truncate.js to check whether this approach is viable. Your current approach doesn't truncate correctly, and there is some inconsistency to actual output.

[brandonhenry]

@shubham1206agra alrighty, did you have any examples of input that you saw weird output? I tested thoroughly and wasn't able to get anything weird, so just curious

[shubham1206agra]

@brandonhenry This is one specific example. The blockquote is changing the truncation of text.

[brandonhenry]

@shubham1206agra do you actually notice any markdown breaking though?

I did some digging here and I noticed that i had implemented this not how we want this to be.

I updated and things are working very consistent. The steps are:

1. Convert html to markdown
2. Trim markdown based only on letters, spaces, and new lines
3. Convert trimmed markdown to html and return.

check it out now and let me know thoughts

I tested with these inputs:

# Welcome to StackEdit!

Hi! I'm your first Markdown file in **StackEdit**. If you want to learn about StackEdit, you can read me. If you want to play with Markdown, you can edit me. Once you have finished with me, you can create new files by opening the **file explorer** on the left corner of the navigation bar.




earn about StackEdit, you can read me. If you want to play with Markdown, you can edit me. Once you have finished with me, you can create new files by opening the **f
joijdoiwedwe

The Markdown philosophy emphasizes readability and simplicity. It allows you to write plain text that can be easily converted to HTML. Markdown supports various elements such as:

- Headers
- Paragraphs
- Lists (ordered and unordered)
- Links
- Images
- Blockquotes
- Code blocks

Markdown uses simple and intuitive syntax, making it accessible to writers of all levels. Its widespread adoption has made it a staple in the world of content creation and documentation.

[brandonhenry]

@shubham1206agra i added comments to the function so you can better follow my logic and we can work together to resolve this one 👍🏿

[brandonhenry]

looking over this again, we probably don't even need to convert to markdown at all. We should be able to use the exact text that is in the text input because that is the markdown, conversion seems redundant

Edit: i see now why we need to first convert. the room details page and the edit description page are different, so we actually don't have access to the original markdown that's used on the details page. Thats why we need to convert from html to markdown (which the user can only ever enter markdown anyway so this should be fine)

[shubham1206agra]

@brandonhenry Is there any reason why you are not using html for truncation directly? I don't think there's a need for conversion to markdown for truncation.

[brandonhenry]

@shubham1206agra mainly because

1. this was my proposal route
2. what user enters is markdown, so i thought it best to truncate based on what user has entered rather than the converted html

Edit: In addition, there are more rules to account for when converting to HTML, in my opinion. We would probably just want to use a package at that point. I did some more digging on trying to do a truncateHTML function, and it would be too much code to account for, imo 🤔 What are your thoughts?

Should we:

1. convert to markdown.
2. try something an approach that matches our replace function and truncates the HTML directly like this:

function truncateHTML(htmlText: string, maxLength: number, addEllipsis = true) {
    if (htmlText.length <= maxLength) {
        return htmlText;
    }

    let truncatedHtml = '';
    let currentLength = 0;
    let insideTag = false;
    let insideSpecialTag = false;
    const tagStack = [];

    const isSpecialTag = (tag: string) => {
        const specialTags = ['code', 'pre', 'a', 'img', 'mention-here', 'mention-user', 'emoji'];
        return specialTags.some((specialTag) => tag.startsWith(`<${specialTag}`));
    };

    const processSpecialTag = (tag: string) => {
        const tagName = tag.match(/<(\w+)/)?.[1];
        if (tagName) {
            const closingTag = `</${tagName}>`;
            const index = htmlText.indexOf(closingTag, currentLength);
            if (index !== -1) {
                const content = htmlText.slice(currentLength, index + closingTag.length);
                truncatedHtml += content;
                currentLength = index + closingTag.length;
                return;
            }
        }
        truncatedHtml += tag;
        currentLength += tag.length;
    };

    const processTag = (tag: string) => {
        if (isSpecialTag(tag)) {
            processSpecialTag(tag);
        } else {
            const tagName = tag.match(/<\/?(\w+)/)?.[1];
            if (tag.startsWith('</')) {
                const lastOpenTag = tagStack.pop();
                if (lastOpenTag !== tagName) {
                    tagStack.push(lastOpenTag!);
                }
            } else if (!tag.endsWith('/>')) {
                tagStack.push(tagName!);
            }
            truncatedHtml += tag;
            currentLength += tag.length;
        }
    };

    const processText = (text: string) => {
        const remainingLength = maxLength - currentLength;
        if (remainingLength <= 0) {
            return;
        }
        const truncatedText = text.slice(0, remainingLength);
        truncatedHtml += truncatedText;
        currentLength += truncatedText.length;
    };

    const parser = new ExpensiMark();
    const tokens = parser.tokenize(htmlText);

    for (const token of tokens) {
        if (insideSpecialTag) {
            truncatedHtml += token.value;
            currentLength += token.value.length;
            if (token.type === 'tag' && token.value.startsWith('</')) {
                insideSpecialTag = false;
            }
        } else if (token.type === 'tag') {
            if (isSpecialTag(token.value)) {
                insideSpecialTag = true;
            }
            processTag(token.value);
        } else {
            processText(token.value);
        }

        if (currentLength >= maxLength) {
            break;
        }
    }

    while (tagStack.length > 0) {
        const tagName = tagStack.pop();
        truncatedHtml += `</${tagName}>`;
    }

    if (addEllipsis && currentLength >= maxLength) {
        truncatedHtml += '...';
    }

    return truncatedHtml;
}

3. or just find a reliable node package?

Cc. @puneetlath @greg-schroeder

[puneetlath]

I think it's fine to truncate the markdown if that's simpler.

[brandonhenry]

cool beans. so what do you think of current implementation @shubham1206agra ?

[brandonhenry]

@shubham1206agra any updates on this one

[puneetlath]

@shubham1206agra can you give an eta on when you'll be able to review?

[brandonhenry]

Happy Monday @shubham1206agra - any updates on this one?

Cc. @puneetlath

[brandonhenry]

any updates @shubham1206agra ?

[brandonhenry]

Anything here? @shubham1206agra

@puneetlath not sure if we should get a new C+ if shub is out

[shubham1206agra]

I am not out

[brandonhenry]

@shubham1206agra I've changed this to not convert to markdown and only convert the html using the function from here

please let me know if this works!

Cc. @puneetlath

[brandonhenry]

any updates @shubham1206agra

[shubham1206agra]

@brandonhenry I have started looking in the implementation. I will update you here with progress.

[brandonhenry]

thanks @shubham1206agra !

[brandonhenry]

@shubham1206agra fixed based on feedback. this is ready for another view

[shubham1206agra]

Please rename the variables and functions correctly and according to guidelines. https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md