hygraph/rich-text

RichText renders empty elements resulting in broken design

rpfaeffle opened this issue · 9 comments

rpfaeffle commented

I've been testing out the <RichText /> and came accross a possible problem which is mainly caused by users providing empty elements in the CMS, but it can cause problems in the design when working with ::after or ::before css selectors or other properties that define the size/appearance of the components.

I have already figured out a solution for this problem, which would be to check whether the element is a media element which can be achieved by checking for a url or src attribute and filtering the children in the element creation process to sort out all text elements containing an empty string.

If you want me to create a pull request for this, I have already written some code to fix this.

PS: A problem could be an empty table cell which needs to be displayed. Because of this, this feature maybe sould be optional.

jpedroschmitz commented

Hey @KaterBasilisk6, glad you have been using the project!

Do you have an image example of how it looks with this bug?

I'd love to review your PR. Let me know if you need anything to set up your environment.

rpfaeffle commented

Hey @jpedroschmitz, thank you for your quick reply.

I do not think it should be marked as a bug, because it's caused by users providing an empty row after, for example the heading which is not converted into a paragraph. It would be an enhancement for users and developers to easily remove those elements on creation if they want to. (Maybe we could provide a default list for empty elements to remove and check if the given element belongs to this list, so the developer can keep empty tags like paragraphs, or table cells but remove a second empty heading which would also cause problems in SEO).

/// Structure of Rich Text content
{
  h2: Lorem ipsum dolor sit amet
  h2:
  p: Lorem ipsum dolor...
}

How it currently looks like:

Screenshot of <RichText /> result

<h2>Lorem ipsum dolor sit amet</h2>
<h2></h2>
<p>Lorem ipsum dolor...</p>

How it's intended to look like:

Screenshot of how it should look like

<h2>Lorem ipsum dolor sit amet</h2>
<p>Lorem ipsum dolor...</p>
jpedroschmitz commented

I see. We should definitely implement this filter in the project.

I like the idea of defining a list of elements you can remove from rendering if empty, but I think it would be better if we remove all heading elements if they have empty text. Empty headings will affect SEO, and we don't want this to happen.

About the other elements, I don't see any problem in rendering if empty. Images, Videos, iframes will never be empty. And elements like blockquote and lists don't need to be removed IMO.

  • 👍1
jpedroschmitz commented

Hey, I've published 0.1.3 on NPM that fixes the issue!

Thanks for your help @KaterBasilisk6 ❤️

  • ❤️1
rpfaeffle commented

You welcome @jpedroschmitz

Endorel commented

Hi!

I'm having a similar issue with rendering empty theads. I get a validateDOMNesting error that says "Whitespace text nodes cannot appear as a child of <thead>." In the editor I have a table that has 1 row and 2 columns, and no header.

Would it be possible to include tables in the rule regulating the rendering of empty elements?

Thanks for this package btw!

jpedroschmitz commented

Hey @Endorel, sorry for the late reply. I missed this notification :(

Yeah, we can! I'll be working on this project for the next few days! I'll let you know when I add this rule =D

jpedroschmitz commented

Hey @Endorel, I just published 0.1.5 that fixes this issue! Thanks a lot for reporting it ❤️

Endorel commented

Great! Thank you for fixing this so quickly! :)

  • ❤️1