Mattermost-server: [Help Wanted] [MM-13297] Web: Remove usage of localizeMessage and dangerouslySetInnerHTML from components in `components/help`

[esethna]

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

---

Notes: Jira ticket

Instead of concatenating the strings, passing them to formatText, and then passing that into a span's dangerouslySetInnerHTML like

const message = [];
message.push(localizeMessage('help.attaching.title', '# Attaching Files\n_____'));

return <span dangerouslySetInnerHTML=/>;

we should remove the markdown from the messages and use proper HTML tags containing FormattedMessage components like

return (
    <h1>
        <FormattedMessage
            id='help.attaching.title'
            defaultMessage='Attaching Files'
        />
    </h1>
);

If that doesn't work, we could instead use FormattedMarkdownMessage components for each line, but removing the markdown entirely should be preferred.

[kosgrz]

I'd like to take this.

[kosgrz]

@esethna I have a problem. Let's say I have this message:

localizeMessage('help.attaching.methods', '## Attachment Methods\nAttach a file...')

It is currently being rendered to this HTML:

<h2 class="markdown__heading">Attachment Methods</h2>
<p>Attach a file...</p>

Of course if I want to remove usage of the localizeMessage method, I cannot just use FormattedMessage component for the whole message, like this:

<FormattedMessage
    id='help.attaching.methods'
    defaultMessage='## Attachment Methods\nAttach a file...'
/>

because result will be:

<span>## Attachment Methods\nAttach a file...</span>

Also I can't use FormattedMarkdownMessage component because there will be no "markdown__heading" class in the h2 tag.

My proposition is to divide the text in the message to be in two separate messages, something like this:

"help.attaching.methods.title":"Attachment Methods",
"help.attaching.methods.description":"Attach a file..."

If I have two separate messages, I will be able to simply write:

<h2 className='markdown__heading'>
    <FormattedMessage
        id='help.attaching.methods.title'
        defaultMessage='Attachment Methods'
    />
</h2>
<FormattedMessage
    id='help.attaching.methods.description'
    defaultMessage='Attach a file...'
/>

Unfortunately this will require to change all translation files, but I haven't better idea. What is your opinion?

[hanzei]

@kosgrz Thanks for your interest in this ticket!

@saturninoabril Could help with the question above?

[saturninoabril]

Thanks @kosgrz @hanzei!

How about make it with two \n, like '## Attachment Methods\n\nAttach a file...'? And use it either localizeMessage or FormattedMarkdownMessage (but preferably FormattedMarkdownMessage).

[kosgrz]

@saturninoabril thanks for the answer!
Unfortunately FormattedMarkdownMessage doesn't support markdowns like \n\n and doesn't add a markdown_heading class to h tags. So the result of the following code:

<FormattedMarkdownMessage
  id='help.attaching.
  defaultMessage={'## Attachment Methods\n\nAttach a file...'}
/>

will be:

<span>
  <h2 id="attachment-methods">Attachment Methods</h2>
  Attach a file...
</span>

As you can see, the h2 tag doesn't have a markdown__heading class. Also, text "Attach a file..." isn't inside a p tag.

Maybe I could create a new component, which will be almost exactly the same as FormattedMarkdownMessage, but will be using different CustomerRenderer with the following definition:

class CustomRenderer extends marked.Renderer {
    link(href, title, text) {
        if (href[0] === TARGET_BLANK_URL_PREFIX) {
            return `<a href="${href.substring(1, href.length)}" target="_blank">${text}</a>`;
        }
        return `<a href="${href}">${text}</a>`;
    }

    heading(text, level) {
        return `<h${level} class="markdown__heading">${text}</h${level}>`;
    }

    paragraph(text) {
        return `<p>${text}</p>`;
    }
}

What do you think about this idea?

[saturninoabril]

@kosgrz Sorry for I misunderstood and missed to read from the top. The goal is to remove the markdown as much as possible. So your initial proposal above (screenshot below) is what we wanted to achieve. It's also expected to change/break the translation files, so no worry. Please go ahead with your plan.

[hanzei]

Fixed with https://github.com/mattermost/mattermost-webapp/pull/2233.

Thanks @kosgrz! :+1: