Skip to content

Commit

Permalink
fix(react): heading with links not rendered (#41)
Browse files Browse the repository at this point in the history
  • Loading branch information
jpedroschmitz authored Oct 15, 2021
1 parent 4d51023 commit c7ea848
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 62 deletions.
5 changes: 5 additions & 0 deletions .changeset/yellow-shirts-stare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphcms/rich-text-react-renderer': patch
---

Fix heading with links not being rendered
8 changes: 2 additions & 6 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
module.exports = {
extends: [
'react-app',
'prettier/@typescript-eslint',
'plugin:prettier/recommended',
],
plugins: ['testing-library', 'jest-dom'],
extends: ['react-app', 'prettier/@typescript-eslint', 'prettier'],
plugins: ['testing-library', 'jest-dom', 'prettier'],
settings: {
react: {
version: '999.999.999',
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"husky": "^6.0.0",
"lerna": "^3.15.0",
"lint-staged": "^11.0.0",
"prettier": "^2.3.0",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"size-limit": "^4.10.2",
Expand Down
32 changes: 16 additions & 16 deletions packages/html-to-slate-ast/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from 'slate';
import { jsx } from 'slate-hyperscript';
import { sanitizeUrl } from '@braintree/sanitize-url';
import type { Element, Mark } from '@graphcms/rich-text-types';
import { Element, Mark } from '@graphcms/rich-text-types';

const ELEMENT_TAGS: Record<
HTMLElement['nodeName'],
Expand All @@ -16,7 +16,7 @@ const ELEMENT_TAGS: Record<
OL: () => ({ type: 'numbered-list' }),
UL: () => ({ type: 'bulleted-list' }),
P: () => ({ type: 'paragraph' }),
A: (el) => {
A: el => {
const href = el.getAttribute('href');
if (href === null) return {};
return {
Expand Down Expand Up @@ -44,7 +44,7 @@ const ELEMENT_TAGS: Record<
TR: () => ({ type: 'table_row' }),
TD: () => ({ type: 'table_cell' }),
TH: () => ({ type: 'table_cell' }),
IMG: (el) => {
IMG: el => {
const href = el.getAttribute('src');
const title = Boolean(el.getAttribute('alt'))
? el.getAttribute('alt')
Expand Down Expand Up @@ -108,7 +108,7 @@ function deserialize<
parent = el.childNodes[0];
}
let children = Array.from(parent.childNodes)
.map((c) => deserialize(c, global))
.map(c => deserialize(c, global))
.flat();

if (children.length === 0) {
Expand All @@ -124,7 +124,7 @@ function deserialize<
if (
isElementNode(el) &&
Array.from(el.attributes).find(
(attr) => attr.name == 'role' && attr.value === 'heading'
attr => attr.name === 'role' && attr.value === 'heading'
)
) {
const level = el.attributes.getNamedItem('aria-level')?.value;
Expand Down Expand Up @@ -190,7 +190,7 @@ function deserialize<
children: [{ text: '' }],
},
]
: childNodes.map((child) => ({
: childNodes.map(child => ({
type: 'paragraph',
children: [{ text: child.textContent ? child.textContent : '' }],
}));
Expand All @@ -204,7 +204,7 @@ function deserialize<
if (nodeName === 'DIV') {
const childNodes = Array.from(el.childNodes);
const isParagraph = childNodes.every(
(child) =>
child =>
(isElementNode(child) && isInlineElement(child)) || isTextNode(child)
);
if (isParagraph) {
Expand Down Expand Up @@ -240,36 +240,36 @@ function deserialize<
})();
if (tagNames) {
const attrs = tagNames.reduce((acc, current) => {
return ({...acc, ...TEXT_TAGS[current]() });
return { ...acc, ...TEXT_TAGS[current]() };
}, {});
return children.map((child) => {
return children.map(child => {
if (typeof child === 'string') {
return jsx('text', attrs, child);
}

if (isChildNode(child, global)) return child;

if (SlateElement.isElement(child) && !SlateText.isText(child)) {
child.children = child.children.map((c) => ({ ...c, ...attrs }));
child.children = child.children.map(c => ({ ...c, ...attrs }));
return child;
}

return child;
});
}
}

if (TEXT_TAGS[nodeName]) {
const attrs = TEXT_TAGS[nodeName](el as HTMLElement);
return children.map((child) => {
return children.map(child => {
if (typeof child === 'string') {
return jsx('text', attrs, child);
}

if (isChildNode(child, global)) return child;

if (SlateElement.isElement(child) && !SlateText.isText(child)) {
child.children = child.children.map((c) => ({ ...c, ...attrs }));
child.children = child.children.map(c => ({ ...c, ...attrs }));
return child;
}

Expand Down Expand Up @@ -441,7 +441,7 @@ export async function htmlToSlateAST(html: string) {
const domDocument = await parseDomDocument(normalizedHTML);
const global = await (async () => {
if (typeof window !== 'undefined') return window;
return await import('jsdom').then((jsdom) => new jsdom.JSDOM().window);
return await import('jsdom').then(jsdom => new jsdom.JSDOM().window);
})();
return deserialize(domDocument.body, global);
}
Expand Down
31 changes: 18 additions & 13 deletions packages/html-to-slate-ast/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ test('Transforms top level span into paragraph', () => {
in fact, is the very CSS and HTML rendered as I type this blog. There are calls to HTML element classes that style
certain properties. For example, the font-family properties in the ".postArticle-content .graf — p" class has a serif
font value, hence the text rendered in this article is of the serif family. All this to say, if you as a pro</span>`).then(
(ast) =>
ast =>
expect(ast).toEqual([
{
type: 'paragraph',
Expand All @@ -30,7 +30,7 @@ test('Transforms inner span into paragraph', () => {
in fact, is the very CSS and HTML rendered as I type this blog. There are calls to HTML element classes that style
certain properties. For example, the font-family properties in</span><span> the ".postArticle-content .graf — p" class has a serif
font value, hence the text rendered in this article is of the serif family. All this to say, if you as a pro</span></p>`).then(
(ast) =>
ast =>
expect(ast).toEqual([
{
type: 'paragraph',
Expand All @@ -49,7 +49,7 @@ test('Transforms inner span into paragraph', () => {

test('Transforms inner spans wrapped in a div into paragraph', () => {
const input = fs.readFileSync(__dirname + '/html_input.html').toString();
return htmlToSlateAST(input).then((ast) =>
return htmlToSlateAST(input).then(ast =>
expect(ast).toStrictEqual([
{
type: 'paragraph',
Expand Down Expand Up @@ -91,7 +91,7 @@ test('Transforms Google Docs input', () => {
const input = fs
.readFileSync(__dirname + '/google-docs_input.html')
.toString();
return htmlToSlateAST(input).then((ast) =>
return htmlToSlateAST(input).then(ast =>
expect(ast).toEqual([
{
type: 'heading-one',
Expand Down Expand Up @@ -362,12 +362,14 @@ test('Transforms Google Docs input', () => {
children: [
{
type: 'link',
href: 'https://lh6.googleusercontent.com/TkJFBZvkyXTa602F0gkp2phU0O1eHu96RdKFcQ8l_EOS_CBfcI9jYRixN6sNRFnFiZ-ssbLbnLDReb3FrEZ1MnLr70c5gIvPmhJtV7appyVEDSeHLIRdNwdNzbIqs3l2GOgGLGC5=s0',
href:
'https://lh6.googleusercontent.com/TkJFBZvkyXTa602F0gkp2phU0O1eHu96RdKFcQ8l_EOS_CBfcI9jYRixN6sNRFnFiZ-ssbLbnLDReb3FrEZ1MnLr70c5gIvPmhJtV7appyVEDSeHLIRdNwdNzbIqs3l2GOgGLGC5=s0',
title: 'Screenshot 2021-06-10 at 15.56.22.png',
openInNewTab: true,
children: [
{
text: 'https://lh6.googleusercontent.com/TkJFBZvkyXTa602F0gkp2phU0O1eHu96RdKFcQ8l_EOS_CBfcI9jYRixN6sNRFnFiZ-ssbLbnLDReb3FrEZ1MnLr70c5gIvPmhJtV7appyVEDSeHLIRdNwdNzbIqs3l2GOgGLGC5=s0',
text:
'https://lh6.googleusercontent.com/TkJFBZvkyXTa602F0gkp2phU0O1eHu96RdKFcQ8l_EOS_CBfcI9jYRixN6sNRFnFiZ-ssbLbnLDReb3FrEZ1MnLr70c5gIvPmhJtV7appyVEDSeHLIRdNwdNzbIqs3l2GOgGLGC5=s0',
},
],
},
Expand All @@ -388,7 +390,7 @@ test('Transforms Google Docs input', () => {
test('Converts word documents', () => {
return htmlToSlateAST(
fs.readFileSync(__dirname + '/word-document.html').toString()
).then((ast) =>
).then(ast =>
expect(ast).toStrictEqual([
{
type: 'heading-one',
Expand Down Expand Up @@ -580,16 +582,18 @@ test('Converts word documents', () => {
test('Converts an image pasted from Google Docs into a link node', () => {
return htmlToSlateAST(
fs.readFileSync(__dirname + '/image.html').toString()
).then((ast) =>
).then(ast =>
expect(ast).toStrictEqual([
{
type: 'link',
href: 'https://lh5.googleusercontent.com/EqByyE2l_VVSU6KoXFlkpPjJIBsbMTb4Dkr0cuvy2K5ctn8BoJsDHBXO0rU2wyck72_ZF1rqJ5kJ0iMEjU4Jwf7mKhRaLWoHJAzX5WvpfMytIR9sw3EwBcdQdRlIwSrsQ3odhUYq',
href:
'https://lh5.googleusercontent.com/EqByyE2l_VVSU6KoXFlkpPjJIBsbMTb4Dkr0cuvy2K5ctn8BoJsDHBXO0rU2wyck72_ZF1rqJ5kJ0iMEjU4Jwf7mKhRaLWoHJAzX5WvpfMytIR9sw3EwBcdQdRlIwSrsQ3odhUYq',
title: "this is this image's title",
openInNewTab: true,
children: [
{
text: 'https://lh5.googleusercontent.com/EqByyE2l_VVSU6KoXFlkpPjJIBsbMTb4Dkr0cuvy2K5ctn8BoJsDHBXO0rU2wyck72_ZF1rqJ5kJ0iMEjU4Jwf7mKhRaLWoHJAzX5WvpfMytIR9sw3EwBcdQdRlIwSrsQ3odhUYq',
text:
'https://lh5.googleusercontent.com/EqByyE2l_VVSU6KoXFlkpPjJIBsbMTb4Dkr0cuvy2K5ctn8BoJsDHBXO0rU2wyck72_ZF1rqJ5kJ0iMEjU4Jwf7mKhRaLWoHJAzX5WvpfMytIR9sw3EwBcdQdRlIwSrsQ3odhUYq',
},
],
},
Expand All @@ -600,7 +604,7 @@ test('Converts an image pasted from Google Docs into a link node', () => {
test('Reshape an incorrectly structured table', () => {
return htmlToSlateAST(
'<table><colgroup><col /><col /></colgroup><tbody><tr><td></td></tr><tr></tr></tbody></table>'
).then((ast) =>
).then(ast =>
expect(ast).toStrictEqual([
{
type: 'table',
Expand Down Expand Up @@ -654,13 +658,14 @@ test('Reshape an incorrectly structured table', () => {

test('Transforms pre tags into code-block nodes', () => {
const input = fs.readFileSync(__dirname + '/pre.html').toString();
return htmlToSlateAST(input).then((ast) =>
return htmlToSlateAST(input).then(ast =>
expect(ast).toStrictEqual([
{
type: 'code-block',
children: [
{
text: " L TE\n A A\n C V\n R A\n DOU\n LOU\n REUSE\n QUE TU\n PORTES\n ET QUI T'\n ORNE O CI\n VILISÉ\n OTE- TU VEUX\n LA BIEN\n SI RESPI\n RER - Apollinaire",
text:
" L TE\n A A\n C V\n R A\n DOU\n LOU\n REUSE\n QUE TU\n PORTES\n ET QUI T'\n ORNE O CI\n VILISÉ\n OTE- TU VEUX\n LA BIEN\n SI RESPI\n RER - Apollinaire",
},
],
},
Expand Down
11 changes: 5 additions & 6 deletions packages/react-renderer/src/RichText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from './defaultElements';
import { RenderText } from './RenderText';
import { getElements } from './util/getElements';
import { elementIsEmpty } from './util/elementIsEmpty';

function RenderNode({
node,
Expand Down Expand Up @@ -65,15 +66,15 @@ function RenderElement({
const { nodeId, nodeType } = rest;

/**
* Checks if element has empty text, so it can be removed.
* Checks if the element is empty, so that it can be removed.
*
* Elements that can be removed with empty text are defined in `defaultRemoveEmptyElements`
*/
if (
defaultRemoveEmptyElements?.[
elementKeys[type] as keyof RemoveEmptyElementType
] &&
children[0].text === ''
elementIsEmpty({ children })
) {
return <Fragment />;
}
Expand All @@ -85,7 +86,7 @@ function RenderElement({
* Since there won't be duplicated ID's, it's safe to use the first element.
*/
const referenceValues = isEmbed
? references?.filter((ref) => ref.id === nodeId)[0]
? references?.filter(ref => ref.id === nodeId)[0]
: null;

/**
Expand Down Expand Up @@ -251,9 +252,7 @@ export function RichText({
If it isn't defined and there's embed elements, it will show a warning
*/
if (__DEV__) {
const embedElements = elements.filter(
(element) => element.type === 'embed'
);
const embedElements = elements.filter(element => element.type === 'embed');

if (embedElements.length > 0 && !references) {
console.warn(
Expand Down
6 changes: 3 additions & 3 deletions packages/react-renderer/src/defaultElements.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ export const defaultElements: Required<NodeRendererType> = {
),
list_item_child: ({ children }) => <>{children}</>,
Asset: {
audio: (props) => <Audio {...props} url={props.url} />,
image: (props) => <Image {...props} src={props.url} />,
video: (props) => <Video {...props} src={props.url} />,
audio: props => <Audio {...props} url={props.url} />,
image: props => <Image {...props} src={props.url} />,
video: props => <Video {...props} src={props.url} />,
font: FallbackForCustomAsset,
application: FallbackForCustomAsset,
model: FallbackForCustomAsset,
Expand Down
32 changes: 32 additions & 0 deletions packages/react-renderer/src/util/elementIsEmpty.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {
ElementNode,
isElement,
isText,
Text,
} from '@graphcms/rich-text-types';

export function elementIsEmpty({
children,
}: {
children: (ElementNode | Text)[];
}): boolean {
// Checks if the children array has more than one element.
// It may have a link inside, that's why we need to check this condition.
if (children.length > 1) {
const hasText = children.filter(function f(child): boolean | number {
if (isText(child) && child.text !== '') {
return true;
}

if (isElement(child)) {
return (child.children = child.children.filter(f)).length;
}

return false;
});

return hasText.length > 0 ? false : true;
} else if (children[0].text === '') return true;

return true;
}
19 changes: 18 additions & 1 deletion packages/react-renderer/test/RichText.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,28 @@ describe('@graphcms/rich-text-react-renderer', () => {
`);
});

it('renders content correctly if received a object with empty children', () => {
it('should not render elements if received a object with empty children', () => {
const { container } = render(<RichText content={emptyContent} />);

expect(container).toMatchInlineSnapshot(`
<div>
<h2>
<a
href="https://graphcms.com"
>
Testing Link
</a>
</h2>
<h2>
<a
href="https://graphcms.com"
>
Link
</a>
2
</h2>
<table>
<tbody>
<tr>
Expand Down
Loading

0 comments on commit c7ea848

Please sign in to comment.