Skip to content

Commit

Permalink
fix: [IOCOM-2088] Handling of <br /> tags on IOMarkdown (#6679)
Browse files Browse the repository at this point in the history
## Short description
This PR fixes a crash on IOMarkdown when using `<br />` tags
<video
src="https://github.com/user-attachments/assets/ce39215f-1689-4f32-ac7d-645113510ed2"/>

## List of changes proposed in this pull request
**TL;DR**
- AST bottom-to-top traversal in HTML node, in order to detect the
presence of a `Paragraph` node. If absent, one is inserted as a parent
of the `Fragment` tag.
- Html node support has been removed from preconditions, message and
service details, in order to have a better layout compatibility with
existing data (since it was adding extra spaces when the source used
both `<br />` and newline characters to produce a newline)

**Detailed version**
The following markdown
```
Continuando

<br />
```
is parsed into the following AST
```
Paragraph 
--Str
Spacer
Html
```
which is later converted to
````
<Body><Fragment>Continuando</Fragment></Body>
<VSpacer />
<Fragment>\n</Fragment>
````
Giving the runtime error `Text strings must be rendered within a <Text>
component`, due to the `\n` character being wrapped inside a mere
Fragment.

With this solution, when an HTML node is found, the AST is travelled
backwards towards the root and if no Paragraph node is found, the
Fragment is wrapped inside a `<Body>` component.

Another approach was to replace the Fragment with a Body but this means
that any `<br />` tag inside a standard text (like `this is<br />an
example`) would have been converted into a double nested `<Body>`.

Moreover, `<br />` support has been removed from preconditions, message
and service details, since there are existing (and immutable) cases
where the markdown contains both `<br />` and newline characters to
handle a new line. In such cases, the new component rendered both
newlines, causing a wide vertical spacing.

## How to test
Using the io-dev-api-server, run the application and check that the
markdown reported above is handled properly.

Co-authored-by: Martino Cesari Tomba <[email protected]>
  • Loading branch information
Vangaorth and forrest57 authored Feb 5, 2025
1 parent 94eab6a commit 5e79c2c
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 6 deletions.
14 changes: 14 additions & 0 deletions ts/components/IOMarkdown/markdownRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ export const isTxtLinkNode = (node: TxtNode): node is TxtLinkNode =>
node.type === "Link";
export const isTxtStrNode = (node: TxtNode): node is TxtStrNode =>
node.type === "Str";
export const isTxtParagraphNode = (node: TxtNode): node is TxtParagraphNode =>
node.type === "Paragraph";

export type LinkData = {
text: string;
Expand Down Expand Up @@ -204,3 +206,15 @@ export const extractLinkDataFromRootNode = (
extractLinkDataFromRootNode(node, links);
}
});

export const isParagraphNodeInHierarchy = (
input: TxtNode | undefined
): boolean => {
if (input == null || input.parent == null) {
return false;
} else if (isTxtParagraphNode(input)) {
return true;
}

return isParagraphNodeInHierarchy(input.parent);
};
15 changes: 13 additions & 2 deletions ts/components/IOMarkdown/renderRules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ import I18n from "../../i18n";
import { openWebUrl } from "../../utils/url";
import { isAndroid } from "../../utils/platform";
import { IOMarkdownRenderRules, Renderer } from "./types";
import { extractAllLinksFromRootNode, LinkData } from "./markdownRenderer";
import {
extractAllLinksFromRootNode,
isParagraphNodeInHierarchy,
LinkData
} from "./markdownRenderer";

const BULLET_ITEM_FULL = "\u2022";
const BULLET_ITEM_EMPTY = "\u25E6";
Expand Down Expand Up @@ -441,7 +445,14 @@ export const DEFAULT_RULES: IOMarkdownRenderRules = {
const [, value] = val;

if (value === "br") {
return <Fragment key={getTxtNodeKey(html)}>{"\n"}</Fragment>;
const hasAParentParagraphNode = isParagraphNodeInHierarchy(html.parent);
return hasAParentParagraphNode ? (
<Fragment key={getTxtNodeKey(html)}>{"\n"}</Fragment>
) : (
<Body key={getTxtNodeKey(html)}>
<Fragment>{"\n"}</Fragment>
</Body>
);
}

return null;
Expand Down
14 changes: 12 additions & 2 deletions ts/features/common/components/IOMarkdown/customRules.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { Fragment } from "react";
import { TxtHeaderNode, TxtLinkNode } from "@textlint/ast-node-types";
import {
TxtHeaderNode,
TxtHtmlNode,
TxtLinkNode
} from "@textlint/ast-node-types";
import { Body, IOToast, MdH1, MdH2, MdH3 } from "@pagopa/io-app-design-system";
import { isIoInternalLink } from "../../../../components/ui/Markdown/handlers/link";
import { handleInternalLink } from "../../../../utils/internalLink";
Expand Down Expand Up @@ -73,5 +77,11 @@ export const generateMessagesAndServicesRules = (
{link.children.map(render)}
</Body>
);
}
},
Html: (_html: TxtHtmlNode) => undefined
});

export const generatePreconditionsRules =
(): Partial<IOMarkdownRenderRules> => ({
Html: (_html: TxtHtmlNode) => undefined
});
8 changes: 6 additions & 2 deletions ts/features/messages/components/Home/PreconditionsContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
import { trackDisclaimerLoadError } from "../../analytics";
import IOMarkdown from "../../../../components/IOMarkdown";
import { isIOMarkdownEnabledOnMessagesAndServicesSelector } from "../../../common/store/reducers";
import { generatePreconditionsRules } from "../../../common/components/IOMarkdown/customRules";
import { PreconditionsFeedback } from "./PreconditionsFeedback";

export const PreconditionsContent = () => {
Expand Down Expand Up @@ -68,12 +69,15 @@ const PreconditionsContentMarkdown = () => {
},
[dispatch, store]
);

if (!markdown) {
return null;
}
return useIOMarkdown ? (
<IOMarkdown content={markdown} onError={onErrorCallback} />
<IOMarkdown
content={markdown}
onError={onErrorCallback}
rules={generatePreconditionsRules()}
/>
) : (
<MessageMarkdown
loadingLines={7}
Expand Down

0 comments on commit 5e79c2c

Please sign in to comment.