Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Document Outline: Exception caused by headings with formatting #8204

Conversation

jorgefilipecosta
Copy link
Member

To display the RichText content that comes from the headings content attribute we should use the RichText.Content instead of rendering it directly.

Fixes: #8160

How has this been tested?

I added a heading to a post.
I made it bold.
I checked the heading appears correctly on the document outline.

To display the RichText content that comes from the headings we should use the RichText.Content component.
@jorgefilipecosta jorgefilipecosta force-pushed the fix/bug-on-document-outline-if-headings-contain-formatting branch from 855d03c to 337a1cb Compare July 25, 2018 18:59
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes the issue for me. Thanks Jorge!

@jorgefilipecosta jorgefilipecosta merged commit aac31cb into master Jul 26, 2018
@jorgefilipecosta jorgefilipecosta deleted the fix/bug-on-document-outline-if-headings-contain-formatting branch July 26, 2018 11:12
@noisysocks noisysocks added this to the 3.4 milestone Jul 27, 2018
@chrisvanpatten chrisvanpatten mentioned this pull request Jul 27, 2018
@mcsf
Copy link
Contributor

mcsf commented Nov 20, 2018

Should we strip the formatting rather than just pass it along?

Case in point: a post converted from classic to blocks (I used the post provided by @nosolosw in #12029) includes a Heading block with some inline styles:

Source Output
screenshot 2018-11-20 at 15 17 44 screenshot 2018-11-20 at 15 17 52

In itself this could be considered a conversion issue, or not. Regardless, this is what Document Outline then looks like:

screenshot 2018-11-20 at 15 18 00

Locally, this is the change I needed to strip the formatting using create and getTextContent from @wordpress/rich-text:

diff --git a/packages/editor/src/components/document-outline/index.js b/packages/editor/src/components/document-outline/index.js
index ad2ca5c2b..1f884c8d2 100644
--- a/packages/editor/src/components/document-outline/index.js
+++ b/packages/editor/src/components/document-outline/index.js
@@ -122,7 +123,7 @@ export const DocumentOutline = ( { blocks = [], title, onSelect, isTitleSupporte
 								emptyHeadingContent :
 								<RichText.Content
 									tagName="span"
-									value={ item.attributes.content }
+									value={ getTextContent( create( { html: item.attributes.content } ) ) }
 								/>
 							}
 							{ isIncorrectLevel && incorrectLevelContent }

jorgefilipecosta added a commit that referenced this pull request Nov 21, 2018
After #8204 was merged, we started using the heading content as the outline item.
So if the heading contained custom formats, they were shown in the document outline. Showing the formats in the outline may break the design.

This commit makes sure we show the plain text in the hading without any formats.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants