Skip to content

Commit

Permalink
fix(atomic): broken HTML because of formatting in CRGA markdown headi…
Browse files Browse the repository at this point in the history
…ng (#4522)

[SVCC-4245](https://coveord.atlassian.net/browse/SVCC-4245)

A CRGA model generally generates markdown headings like this:
```
# Title
```

However, in some cases, it can generate headings this way:
```
**Title**
=====
```

When it does, the HTML returned by the `generated-answer` component is
broken and the heading reads like this:
```
Title>Title
```

The core of the issue is in [this custom Marked
renderer](https://github.com/coveo/ui-kit/blob/master/packages/atomic/src/components/common/generated-answer/generated-content/markdown-utils.ts#L50).
When the heading has formatting, the `text` parameter contains the HTML
rendering of the heading content. Inserting non-escaped HTML into the
`aria-label` attribute then breaks the `div` element.

This PR fixes the issue by affecting only the heading text content to
the `aria-label` attribute. To do so, the HTML elements are removed from
the heading content when setting `aria-label`. The heading content HTML
is kept intact when setting the heading inner HTML though in order to
keep the heading formatting.

For example:
```
**Title**
=====
```
Would generate this HTML
```
<div part="answer-heading-1" aria-label="Title">
  <strong part="answer-strong">Title</strong>
</div>
```

[SVCC-4245]:
https://coveord.atlassian.net/browse/SVCC-4245?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
lbergeron authored Oct 16, 2024
1 parent 836ef3a commit 9e15c6c
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,26 @@ describe('markdownUtils', () => {
);
});
});

it('should transform formatted heading', () => {
const text = '# **bold** *emphasized* with `code` title';

const html = transformMarkdownToHtml(text);

expect(removeLineBreaks(html)).toBe(
'<div part="answer-heading-1" aria-label="bold emphasized with code title"><strong part="answer-strong">bold</strong> <em part="answer-emphasis">emphasized</em> with <code part="answer-inline-code">code</code> title</div>'
);
});

it('should transform heading with nested formatting', () => {
const text = '# ***bold** emphasized with `code`* title';

const html = transformMarkdownToHtml(text);

expect(removeLineBreaks(html)).toBe(
'<div part="answer-heading-1" aria-label="bold emphasized with code title"><em part="answer-emphasis"><strong part="answer-strong">bold</strong> emphasized with <code part="answer-inline-code">code</code></em> title</div>'
);
});
});

describe('with unclosed inline elements in text', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import {marked} from 'marked';

const toInlinePlainText = (textWithHtml: string): string => {
const withoutHtmlTags = textWithHtml.replace(/<[^>]*>/g, ' ');
const withCollapsedWhitespaces = withoutHtmlTags.replace(/\s{2,}/g, ' ');

return withCollapsedWhitespaces.trim();
};

const unclosedElement = /(\*{1,3}|`)($|\w[\w\s]*$)/;

const completeUnclosedElement = (text: string) => {
Expand Down Expand Up @@ -48,7 +55,9 @@ const customRenderer = {
},

heading(text: string, level: number) {
return `<div part="answer-heading-${level}" aria-label="${text}">${text}</div>`;
const plainText = toInlinePlainText(text);

return `<div part="answer-heading-${level}" aria-label="${plainText}">${text}</div>`;
},

html(text: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ describe('c/markdownUtils', () => {
);
});

it('should transform markdown heading with formatting to HTML <div>', () => {
const text = '# **bold** and *emphasized* title';
const result = transformMarkdownToHtml(text, marked);
expect(removeLineBreaks(result)).toEqual(
'<div data-level="answer-heading-1" aria-label="bold and emphasized title"><strong>bold</strong> and <em>emphasized</em> title</div>'
);
});

it('should transform markdown list item to HTML <li>', () => {
const text = '- Hello, world!';
const result = transformMarkdownToHtml(text, marked);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,18 @@ import MARKED_JS from '@salesforce/resourceUrl/marked';
// @ts-ignore
import {loadScript} from 'lightning/platformResourceLoader';

/**
* Transforms a single line of text that may contain HTML to plain text.
* @param {string} textWithHtml A single line of text that may contain HTML
* @returns {string} The value as plain text
*/
const toInlinePlainText = (textWithHtml) => {
const withoutHtmlTags = textWithHtml.replace(/<[^>]*>/g, ' ');
const withCollapsedWhitespaces = withoutHtmlTags.replace(/\s{2,}/g, ' ');

return withCollapsedWhitespaces.trim();
};

// Any number of `*` between 1 and 3, or a single backtick, followed by a word character or whitespace character
const unclosedElement = /(\*{1,3}|`)($|\w[\w\s]*$)/;

Expand Down Expand Up @@ -49,7 +61,9 @@ const customRenderer = {
* @param {string} level
*/
heading(text, level) {
return `<div data-level="answer-heading-${level}" aria-label="${text}">${text}</div>`;
const plainText = toInlinePlainText(text);

return `<div data-level="answer-heading-${level}" aria-label="${plainText}">${text}</div>`;
},

/**
Expand Down

0 comments on commit 9e15c6c

Please sign in to comment.